Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python interpreter #617

Merged
merged 16 commits into from
Aug 13, 2024
13 changes: 11 additions & 2 deletions docs/genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/src/components/BuiltinTools.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<LinkCard title="fs_read_file" description="Reads a file as text from the file system." href="/genaiscript/reference/scripts/system#systemfs_read_file" />
<LinkCard title="fs_read_summary" description="Reads a summary of a file from the file system." href="/genaiscript/reference/scripts/system#systemfs_read_summary" />
<LinkCard title="math_eval" description="Evaluates a math expression" href="/genaiscript/reference/scripts/system#systemmath" />
<LinkCard title="python_interpreter" description="Executes python 3.12 code in a docker container. The process output is returned. Use 'print' to output data." href="/genaiscript/reference/scripts/system#systempython_interpreter" />

Check failure on line 13 in docs/src/components/BuiltinTools.mdx

View workflow job for this annotation

GitHub Actions / build

The link provided for the 'python_interpreter' tool is incorrect. It should point to the correct anchor in the system documentation.

Choose a reason for hiding this comment

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

The link provided for the 'python_interpreter' tool should be consistent with other links, using 'system' as the prefix in the href attribute.

generated by pr-docs-review-commit link_format

<LinkCard title="retrieval_fuzz_search" description="Search for keywords using the full text of files and a fuzzy distance." href="/genaiscript/reference/scripts/system#systemretrieval_fuzz_search" />
<LinkCard title="retrieval_vector_search" description="Search files using embeddings and similarity distance." href="/genaiscript/reference/scripts/system#systemretrieval_vector_search" />
<LinkCard title="retrieval_web_search" description="Search the web for a user query using Bing Search." href="/genaiscript/reference/scripts/system#systemretrieval_web_search" />
Expand Down
67 changes: 67 additions & 0 deletions docs/src/content/docs/reference/scripts/system.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,73 @@
`````


### `system.python_interpreter`

Python Dockerized code execution



- tool `python_interpreter`: Executes python 3.12 code in a docker container. The process output is returned. Use 'print' to output data.

`````js wrap title="system.python_interpreter"
system({
title: "Python Dockerized code execution",
})

const image = env.vars.pythonImage ?? "python:3.12"

Check failure on line 606 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The usage of 'env.vars.pythonImage' is not explained or documented, which could lead to confusion about where this variable is set or how to use it.

Choose a reason for hiding this comment

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

The usage of 'env.vars.pythonImage' is not explained or documented, which could lead to confusion about where this variable is set or how to use it.

generated by pr-docs-review-commit env_usage


let container = null

defTool(
"python_interpreter",
"Executes python 3.12 code in a docker container. The process output is returned. Use 'print' to output data.",
{
type: "object",
properties: {
requirements: {
type: "string",
description: `list of pip packages to install using pip. should be using the pip install format:
<package1>
<package2>

Choose a reason for hiding this comment

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

The code block for the 'requirements' description should be formatted as a code snippet for clarity.

generated by pr-docs-review-commit code_format

`,

Check failure on line 621 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The description for 'requirements' property in the 'python_interpreter' tool is incorrectly formatted. It should be a single line description without line breaks.

Choose a reason for hiding this comment

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

The description for 'requirements' property in the 'python_interpreter' tool is incorrectly formatted. It should be a single line description without line breaks.

generated by pr-docs-review-commit description_format

},
main: {
type: "string",
description: "python 3.12 source code to execute",
},
},
required: ["requirements", "main"],
},
async (args) => {
const { requirements, main = "" } = args
console.log(`python: running code...`)
container = await host.container({ image, networkEnabled: true })
if (requirements) {
console.log(`installing: ` + requirements)
await container.writeText(
"requirements.txt",
requirements.replace(/[ ,]\s*/g, "\n")

Choose a reason for hiding this comment

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

The regular expression pattern used in the 'requirements.txt' replacement should escape the period to match a literal period, not any character.

generated by pr-docs-review-commit regex_pattern

)
const res = await container.exec("pip", [
"install",
"--root-user-action",
"ignore",
"-r",
"requirements.txt",
])
if (res.failed) throw new Error(`Failed to install requirements`)
}

console.log(`code: ` + main)
await container.writeText("main.py", main)
const res = await container.exec("python", ["main.py"])
return res
}

Check failure on line 654 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The 'defTool' function is defined as 'async' but does not use 'await' within its body, which is likely an error.

Choose a reason for hiding this comment

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

The 'defTool' function is defined as an async function but does not use 'await' with the 'host.container' call, which is likely an asynchronous operation.

generated by pr-docs-review-commit async_function

Choose a reason for hiding this comment

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

The 'defTool' function is defined as 'async' but does not use 'await' within its body, which is likely an error.

generated by pr-docs-review-commit async_usage

)

`````


### `system.retrieval_fuzz_search`

Full Text Fuzzy Search
Expand Down
13 changes: 11 additions & 2 deletions genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 47 additions & 4 deletions packages/cli/src/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
import { host } from "../../core/src/host"
import { installImport } from "../../core/src/import"
import { TraceOptions } from "../../core/src/trace"
import { logError, dotGenaiscriptPath } from "../../core/src/util"
import { logError, dotGenaiscriptPath, logVerbose } from "../../core/src/util"
import { CORE_VERSION } from "../../core/src/version"
import { YAMLStringify } from "../../core/src/yaml"
import { isQuiet } from "./log"

type DockerodeType = import("dockerode")

Expand Down Expand Up @@ -62,6 +64,30 @@
this.containers = []
}

async stopContainer(id: string) {
const c = await this._docker?.getContainer(id)
if (c) {
try {
await c.stop()
} catch {}
try {
await c.remove()
} catch (e) {
logError(e)
}
}
const i = this.containers.findIndex((c) => c.id === id)
if (i > -1) {
const container = this.containers[i]
try {
await remove(container.hostPath)
} catch (e) {
logError(e)
}
this.containers.splice(i, 1)
}
}

Check failure on line 89 in packages/cli/src/docker.ts

View workflow job for this annotation

GitHub Actions / build

Missing error handling in the `stopContainer` method. It's recommended to handle errors in the catch block instead of ignoring them.

Choose a reason for hiding this comment

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

Missing error handling in the stopContainer method. There are empty catch blocks which could swallow errors and make debugging difficult.

generated by pr-review-commit missing_error_handling


async checkImage(image: string) {
await this.init()
try {
Expand Down Expand Up @@ -162,6 +188,10 @@
const inspection = await container.inspect()
trace?.itemValue(`container state`, inspection.State?.Status)

const stop: () => Promise<void> = async () => {
await this.stopContainer(container.id)
}

const exec: ShellHost["exec"] = async (
command,
args,
Expand All @@ -178,6 +208,10 @@
trace?.itemValue(`container`, container.id)
trace?.itemValue(`cwd`, cwd)
trace?.item(`\`${command}\` ${args.join(" ")}`)
if (!isQuiet)
logVerbose(
`container exec: ${command} ${args.join(" ")}`
)

let inspection = await container.inspect()
trace?.itemValue(
Expand Down Expand Up @@ -220,8 +254,14 @@
exitCode === 0,
`exit code: ${sres.exitCode}`
)
if (sres.stdout) trace?.detailsFenced(`stdout`, sres.stdout)
if (sres.stderr) trace?.detailsFenced(`stderr`, sres.stderr)
if (sres.stdout) {
trace?.detailsFenced(`stdout`, sres.stdout)
if (!isQuiet) logVerbose(sres.stdout)
}
if (sres.stderr) {
trace?.detailsFenced(`stderr`, sres.stderr)
if (!isQuiet) logVerbose(sres.stderr)
}

return sres
} catch (e) {
Expand All @@ -239,7 +279,9 @@
const writeText = async (filename: string, content: string) => {
const hostFilename = host.path.resolve(hostPath, filename)
await ensureDir(host.path.dirname(hostFilename))
await writeFile(hostFilename, content, { encoding: "utf8" })
await writeFile(hostFilename, content ?? "", {
encoding: "utf8",
})

Choose a reason for hiding this comment

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

Potential null reference error. The content parameter is not checked for null or undefined before being used in writeFile.

generated by pr-review-commit missing_null_check

}

const readText = async (filename: string, content: string) => {
Expand All @@ -262,6 +304,7 @@
disablePurge: !!options.disablePurge,
hostPath,
containerPath,
stop,
exec,
writeText,
readText,
Expand Down
23 changes: 5 additions & 18 deletions packages/cli/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import { emptyDir, ensureDir } from "fs-extra"
import { convertDiagnosticsToSARIF } from "./sarif"
import { buildProject } from "./build"
import { createProgressSpinner } from "./spinner"
import { diagnosticsToCSV } from "../../core/src/ast"
import { CancellationOptions } from "../../core/src/cancellation"
import { ChatCompletionsProgressReport } from "../../core/src/chattypes"
Expand Down Expand Up @@ -109,13 +108,8 @@
const cancellationToken = options.cancellationToken
const jsSource = options.jsSource

const spinner =
!stream && !isQuiet
? createProgressSpinner(`preparing tools in ${process.cwd()}`)
: undefined
const fail = (msg: string, exitCode: number) => {
if (spinner) spinner.fail(msg)
else logVerbose(msg)
logVerbose(msg)
return { exitCode, result }
}

Expand All @@ -130,10 +124,10 @@
const ffs = await host.findFiles(arg, {
applyGitIgnore: excludeGitIgnore,
})
if (!ffs.length) {
if (!ffs?.length) {

Choose a reason for hiding this comment

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

Potential null reference exception. The ffs variable is not checked for null before accessing its length property.

generated by pr-review-commit missing_null_check

return fail(
`no files matching ${arg}`,
FILES_NOT_FOUND_ERROR_CODE

Check failure on line 130 in packages/cli/src/run.ts

View workflow job for this annotation

GitHub Actions / build

Missing error handling for the case when `ffs` is undefined. It's recommended to handle this case to prevent potential runtime errors.

Choose a reason for hiding this comment

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

Missing error handling for the case when ffs is undefined. It's recommended to handle this case to prevent potential runtime errors.

generated by pr-review-commit missing_error_handling

)
}
for (const file of ffs) {
Expand Down Expand Up @@ -188,16 +182,15 @@
infoCb: (args) => {
const { text } = args
if (text) {
if (spinner) spinner.start(text)
else if (!isQuiet) logVerbose(text)
if (!isQuiet) logVerbose(text)
infoCb?.(args)
}
},
partialCb: (args) => {
const { responseChunk, tokensSoFar } = args
tokens = tokensSoFar
if (stream && responseChunk) process.stdout.write(responseChunk)
if (spinner) spinner.report({ count: tokens })
else if (!isQuiet) process.stderr.write(responseChunk)
partialCb?.(args)
},
skipLLM,
Expand Down Expand Up @@ -230,18 +223,13 @@
},
})
} catch (err) {
if (spinner) spinner.fail()
if (isCancelError(err))
return fail("user cancelled", USER_CANCELLED_ERROR_CODE)
logError(err)
return fail("runtime error", RUNTIME_ERROR_CODE)
}
if (!isQuiet) logVerbose("") // force new line
if (spinner) {
if (result.status !== "success")
spinner.fail(`${spinner.text}, ${result.statusText}`)
else spinner.succeed()
} else if (result.status !== "success")
if (result.status !== "success")
logVerbose(result.statusText ?? result.status)

if (outTrace) await writeText(outTrace, trace.content)
Expand Down Expand Up @@ -434,7 +422,6 @@
if (failOnErrors && result.annotations?.some((a) => a.severity === "error"))
return fail("error annotations found", ANNOTATION_ERROR_CODE)

spinner?.stop()
process.stderr.write("\n")
return { exitCode: 0, result }
}
Loading
Loading