Skip to content

Migrate /api/interpreters endpoint from Go to TypeScript#3672

Draft
zackverham wants to merge 8 commits intomainfrom
migrate-interpreters-to-typescript
Draft

Migrate /api/interpreters endpoint from Go to TypeScript#3672
zackverham wants to merge 8 commits intomainfrom
migrate-interpreters-to-typescript

Conversation

@zackverham
Copy link
Collaborator

@zackverham zackverham commented Mar 9, 2026

Summary

Move interpreter detection (Python/R version, lockfiles, package managers) from the Go backend API into a new TypeScript module in the VSCode extension. This eliminates the HTTP round-trip to the Go process for logic that can run directly in TypeScript using child_process.

  • Add src/interpreters/ module with Python and R detection, version constraint parsing, and project metadata reading (.python-version, pyproject.toml, setup.cfg, DESCRIPTION, renv.lock)
  • Replace api.interpreters.get() calls in state.ts with getInterpreterDefaults()
  • Remove GET /api/interpreters Go endpoint and route registration
  • Remove Interpreters API resource class from TypeScript client
  • Add 74 unit tests covering all new modules

Go components intentionally retained

The following Go interpreter code is not removed by this PR because it is still used by other backend endpoints:

Component Used by
InterpretersFromRequest() in api_helpers.go POST /api/deployment (post_deployment.go), POST /api/inspect (post_inspect.go), POST /api/packages/r/scan (post_packages_r_scan.go)
internal/interpreters/ package (Python/R interpreter structs, version detection, requires parsing) InterpretersFromRequest, config.Python.FillDefaults(), config.R.FillDefaults(), bundle manifest generation
config.Python.FillDefaults() / config.R.FillDefaults() in internal/config/types.go Server-side config population during deployment and inspection

These can be migrated in follow-up work as their consuming endpoints are moved to TypeScript.

Type of Change

  • Refactor

Approach

Ported the interpreter detection logic from Go to TypeScript, matching the existing behavior:

  • Python version via python -E -c "import sys; ..." with pyenv shim cache bypass
  • R version via R --version with combined stdout/stderr parsing
  • renv lockfile resolution via renv::paths$lockfile() with fallback to default renv.lock
  • Version constraint adaptation (PEP 440 operators, compatible release ~=)
  • Project metadata parsing for Python requires (.python-version, pyproject.toml, setup.cfg) and R requires (DESCRIPTION, renv.lock)

User Impact

No user-facing behavior change. Interpreter detection now runs in-process in the extension instead of making an HTTP call to the Go backend.

Automated Tests

74 new Vitest unit tests across 6 test files covering:

  • Version constraint adaptation (28 tests)
  • Python requires parsing (10 tests)
  • R requires parsing (10 tests)
  • Python interpreter detection (9 tests)
  • R interpreter detection (11 tests)
  • getInterpreterDefaults() public API (6 tests)

E2E coverage

No new E2E tests are needed. The existing deployment E2E tests (deployments.cy.js, embedded-deployments.cy.js) implicitly validate this migration — they open real Python and R projects, wait for interpreter detection via waitForInterpreterReady(), and deploy to Connect. If interpreter detection regressed (wrong version, missing lockfile, bad package manager), those deployments would fail.

This migration changes internal plumbing (HTTP call to Go → direct child_process execution), not the inputs or outputs. The sidebar displays the same data and configurations receive the same defaults, so the existing E2E suite covers the end-to-end behavior without modification.

Checklist

  • I have updated the root CHANGELOG.md to cover notable changes.

zackverham and others added 6 commits March 9, 2026 15:19
Move interpreter detection (Python/R version, lockfiles, package managers)
from the Go backend API into a new TypeScript module in the VSCode extension.
This eliminates the HTTP round-trip to the Go process for logic that can run
directly in TypeScript using child_process.

- Add src/interpreters/ module with Python and R detection, version constraint
  parsing, and project metadata reading (.python-version, pyproject.toml,
  setup.cfg, DESCRIPTION, renv.lock)
- Replace api.interpreters.get() calls in state.ts with getInterpreterDefaults()
- Remove GET /api/interpreters Go endpoint and route registration
- Remove Interpreters API resource class from TypeScript client
- Add 62 unit tests covering all new modules

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add index.test.ts: 6 tests covering getInterpreterDefaults() including
  rejection fallbacks for Python/R and undefined path handling
- Add rInterpreter.test.ts: R --version with non-zero exit code, lockfile
  path outside projectDir, unparseable renv output, unrecognized R output
- Add pythonInterpreter.test.ts: empty and whitespace-only stdout handling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use const vi.fn() with mockReset() instead of let with reassignment
to avoid TS2348 errors with Vitest v4's Mock type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use vi.hoisted() to declare mocks before vi.mock() hoisting, pass
mock fns directly instead of wrapping in lambdas (fixes TS2348),
and run prettier on all new files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Returns null on invalid input, not empty string.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
expect(result).toBe("");
});

test("does not match tinyR", async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct? Do we not want to match tinyR?

zackverham and others added 2 commits March 9, 2026 16:14
Use Promise.resolve() instead of async arrow function.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Collaborator

@christierney christierney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall shape of this makes sense to me. I didn't review super closely yet since it's in draft.

I have some questions about using the vscode Uri and workspace. Is that a requirement or a convenience? With the config loading code I've tried to keep vscode imports out of it. The extension handles figuring out the workspace root folder and passes that into the "pure" typescript code, which uses node functions for file i/o.

Practically it doesn't make a huge difference since we end up just mocking the vscode stuff, so the tests are still isolated, but it feels worthwhile to me to try to keep the vscode deps to a minimum. I could be talked out of that probably.

// Copyright (C) 2026 by Posit Software, PBC.

import { execFile } from "child_process";
import { Uri } from "vscode";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there something special about the vscode Uri? I would love to avoid importing vscode stuff into our new code wherever possible. (And I'm surprised a node builtin doesn't handle Uri construction)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does have helpers and handles some weird pathing that would make some of this easier I think compared to to just Node path. The URI implementation is also available here: https://github.com/microsoft/vscode-uri outside of VS Code which may be helpful.

},
workspace: {
fs: {
stat: vi.fn(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this used?

@@ -0,0 +1,132 @@
// Copyright (C) 2026 by Posit Software, PBC.

import { Uri, workspace } from "vscode";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question: do we need to depend on vscode for finding files?

Comment on lines +32 to +34
if (preferredPath) {
version = await getPythonVersionFromExecutable(preferredPath, projectDir);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we no longer look for Python in the PATH?

If the user isn't using Positron or the Python extension this will come back with no version. Maybe that is preferred or isn't a big deal though.

// Resolve the renv lockfile path
const lockfilePath = await resolveRenvLockfile(preferredPath!, projectDir);
const lockfileUri = Uri.joinPath(Uri.file(projectDir), lockfilePath);
const lockfilePresent = await fileExists(lockfileUri);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude is flagging this locally as a potential issue. It is describing the lockfilePath as possibly absolute, but Uri.joinPath is always going to give us a relative path.

That makes sense with the code I'm seeing below in resolveRenvLockfile which says The renv lockfile path is absolute; make it relative to projectDir.

We can use path.isAbsolute to check.

We can match what the Go code did, which is use the absolute path to check for the existence of the file and join when it is relative.

): Promise<string> {
const lockfilePath = await getRenvLockfileFromR(rPath, projectDir);
if (lockfilePath) {
// The renv lockfile path is absolute; make it relative to projectDir
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can return the lockfilePath unchanged so sometimes it is absolute.

We should clarify this or adjust it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants