Notify backends of updates to plot render settings

[lionel-]

Addresses #7449.
Addresses #7571.
Addresses #7572.
Closes #7299.

Companion PR: posit-dev/ark#792

This PR is a follow-up to #7247 which implement the possibily for backends to send pre-renderings of plots in their comm_open messages. Currently backends have to use the plot render settings of the last render request. This approach has multiple shortcomings:

• The initial plot is rendered with incorrect settings since the backend hasn't received a render request yet.

• When the user saves a plot with different settings, the backend incorrectly considers these are now the current settings and will use these to render the next plot.

• More importantly, with Jupyter-backed backends, render requests are queued on Shell, which means backends do not receive updated settings as long as code is running in the console. This is problematic when it's a service or long running app that generates the plot. Changing the dimensions of the plot area would mess up the rendering as long as the app is running.

Screen.Recording.2025-05-06.at.17.17.58.mov

Screen.Recording.2025-05-06.at.17.09.20.mov

To fix the first two issues and make progress towards the third, this PR adds a new OpenRPC notification (currently implemented as a request until #7448 is solved) containing plots render settings. It's intended for this notification to be sent "interrupt-style" once we support this (#7447), thus fixing the third issue.

Screen.Recording.2025-05-06.at.17.13.10.mov

Screen.Recording.2025-05-06.at.17.10.30.mov

In the process I've also fixed the issues described in #7571 and #7572:

Screen.Recording.2025-05-06.at.17.33.54.mov

Screen.Recording.2025-05-06.at.17.28.25.mov

We now correctly remember sizing policy changes and no longer rerender unnecessarily when toggling visibility:

Screen.Recording.2025-05-06.at.17.34.15.mov

Screen.Recording.2025-05-06.at.17.29.17.mov

Summary of changes:

• New global event onDidChangePlotsRenderSettings in positron.d.ts, and companion getter for current state getPlotsRenderSettings(). These live in the positron.window namespace and are supported by two new types PlotRenderSettings and PlotRenderFormat at the top-level of positron.

• Note that the naming of the types has been reviewed for consistency and clarity. RenderFormat is now PlotRenderFormat, RenderPolicy

• The positron plots service is now exposed to the extension host to provide the new event and getter to extensions.

• In plotsContainer.tsx we observe changes to the plot container and emit the did-change event with updated settings. This is debounced in the same way we do for console-width changes.

• New subscription mechanism that makes it super easy for backends to receive OpenRPC notifications.

• The comm generation now supports importing types from other comms.

Subscriptions

For backends that support OpenRPC messaging, the new subscription mechanism makes it easy to get notifications as soon as the UI comm is connected, for the inital state as well as future updates. This is only implemented for the plots render settings but we could use the same approach for notifications of console width changes.

UiRuntimeNotifications in positron.d.ts.

The language runtime metadat gain a new field that allow runtimes to subscribe to notifications:

readonly uiSubscriptions?: UiRuntimeNotifications[];

This is currently implemented in the core but I expect that this will be moved to an extension-side middleware in the future (see #4997).

The UI client has a new backend-side notification that can be sent by the frontend (more on this later as this required changes to the comm generators). The goal is to send this notification to subscribed runtimes.

The UI client gains a register() method that makes it possible to tie resources to its lifecycle (in particular it is disposed after a disconnection).

The runtime service provides an event and getter for the connection of the UI client. The event is emitted after each reconnection. A watchUiClient() helper is also provided to call a handler also with the current state, if any, in addition to future states.

The plots service watches UI clients for runtimes that have subscribed to the render settings notifications. As soon as the UI client is connected, the initial notification with current settings is sent and the events are set so that we send notifications for future settings changes.

You can see how this is all plumbed together in the positron plots service:

if (session.runtimeMetadata.uiSubscriptions?.includes(UiRuntimeNotifications.DidChangePlotsRenderSettings)) {
	this._register(this._runtimeSessionService.watchUiClient(session.sessionId, (uiClient) => {
		// Forward future settings updates. Note that the lifecycle of that event
		// handler is tied to the UI client itself, not to the lifecycle of the session.
		uiClient.register(this.onDidChangePlotsRenderSettings(settings => {
			uiClient.didChangePlotsRenderSettings(settings);
		}));

		// Send initial settings immediately
		uiClient.didChangePlotsRenderSettings(this.getPlotsRenderSettings());
	}));
}

In the future I think we could augment the subscriptions with a notification type. For instance the R backend could implement the did-change-console-width handling with a pure R method to be called by the frontend with callMethod().

For plot render settings, we'd want an interrupt-style (#7447) RPC notification handled on the Rust side as this state lives purely in Rust. For console width changes, we really need to update the width global option on the R side, so a method call via Shell OpenRPC (queued alongside execution requests) is the simplest way to implement the handler.

To support this, each notification a backend subscribes to would be associated with a notification kind: Method, Notification, NotificationInterrupt.

Changes to comm generation

I went through a couple of iterations for #7299. At first I duplicated imported types instead of importing them. This turned out to be complicated (dependencies have to be imported too, and these could be external, and there could be circular deps, etc). But also it was impractical. Completions would now suggest two sources for types and languages that don't have interfaces like Rust would need explicit conversion when passing data from a file that imported e.g. from UI into a file that imported from Plot.

So instead external references generate import/use statements in the comm preambles. This requires a two-pass approach, so we lose a bit of the streaming structure of the generators, but I'm not worried about performance here and the import approach makes for cleaner generated code that is easier to use. The first pass is supported by a refVisitor() helper that yields all references in a file. External references are detected and extracted with externalRefComponents(). A higher level collectExternalReferences() helper is used for each comm and returns the imported symbols grouped by external files.

You'll see a big diff in generate-comms.ts that is due to a refactoring I did to support the first approach. I extracted the code that generates data types into separate functions so they could be called again to generate external definitions:

for (const source of contracts) {
	yield* createPythonValueTypes(source, contracts, models);
}
yield* createExternalDefinitions(createPythonValueTypes, contracts, models);

This is no longer needed with the import approach but I kept the extracted functions in the PR as I think it makes the code easier to read and edit.

Release Notes

New Features

• N/A

Bug Fixes

• Fixed more performance issues in the rendering of R plots (Performance for plotting #5184, Unnecessary and incorrect rerenders of plots when toggling visibility on and off #7572). Rendering of R plots should now feel snappier in general, in particular after resizing the sidebar.
• Fixed issue causing incorrect rendering of plots when a new sizing policy is selected (Plot sizing policy is inconsistently remembered #7571).

QA Notes

• Look at the videos to see the difference between a plot that is created with correct render settings right away versus a plot that was created with outdated render settings. In the latter case, the plot visibly goes through two different states in quick succession. Let's call these "rerenders".

• If the plot history strip is set to always visible or never visible, you should see no rerenders as you create plots. If it's set to "auto" though, the default, you should see the the second plot get rerendered because the history strip opens up and invalidates current render settings. From the third plot on, there should be no rerenders.

• Resizing the side bar or panel shouldn't cause rerenders. In particular going from a wide to narrow sidebar or from a tall to short panel shouldn't cause rerenders, i.e. the backend should be notified of render settings that reflect the aspect ratio we use for these problematic viewport sizes.

• Changing the sizing policy (square, landscape, etc) should not cause rerenders for new plots.

• Clearing the plots should not cause rerenders for new plots.

• If you save a plot with different settings, e.g. to tiff before making a new plot, the latter should appear instantaneously. I.e. the request to render a plot for saving shouldn't update the current render settings.

• Hiding the sidebar or panel and revealing it again should not cause a rerender.

@:plots

[github-actions]

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:plots

^{readme  valid tags}

[lionel-]

This is just easier to remember :-)

[lionel-]

I noticed some resource management issue here, we were never cleaning up event listeners. I've added a register() method on plot client instances so that we can tie these resources to the plot lifecycle.

This register() method is used in a more straightforward way below. Here is a bit more involved, we create a disposable object for our resource cleanup. I think that's a bit simpler than using an onDidClose() event whose event listener itself needs to be disposed of along with the client (and not the service as is currently the case).

[lionel-]

This propagation is part of the fix for #7571.

[lionel-]

Part of the fix for #7571.

[lionel-]

We were incorrectly checking the cached value here, comparing by pointer instead of value.

[lionel-]

These guards (here and elsewhere) are part of the fix for #7572.

[lionel-]

We've discussed how to test this with @testlabauto but it will be easier to do it once everything is merged and ark is updated on main as well, so he will send a follow-up PR with the tests (thanks!).

The gist of the tests will be to check that no "render" request was sent to the frontend after doing the following actions:

• Displaying a plot with a fresh instance of Positron.
• Clearing the plots, changing the size of the plots pane, and displaying a plot.
• Toggling the visibility of the plots pane on and off, either quickly or slowly (e.g. 500ms) - the buggy behaviour happened only when toggling slowly.

A slightly hackish but easy way to test this is to check the Console log and verify that no render request was sent to the backend, the string "render" (with quotes) should not appear in the output.