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.

Sign up for GitHub

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
Merged

Python interpreter #617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
41260a9
add python execution system prompt
pelikhan Aug 13, 2024
4612878
add csv tests
pelikhan Aug 13, 2024
af58d36
rename sample
pelikhan Aug 13, 2024
9a9abdd
smaller sample
pelikhan Aug 13, 2024
bb96c01
no cache
pelikhan Aug 13, 2024
3e01249
use default python image
pelikhan Aug 13, 2024
69015f4
use python:3 image
pelikhan Aug 13, 2024
c3f487c
remove spinnger from run
pelikhan Aug 13, 2024
4dabebd
pandas sample
pelikhan Aug 13, 2024
4bf6331
formatting
pelikhan Aug 13, 2024
4561569
escape requirements format
pelikhan Aug 13, 2024
d98d3e7
improving description
pelikhan Aug 13, 2024
e259dd6
simplify requirements format
pelikhan Aug 13, 2024
e67ef5c
pand test
pelikhan Aug 13, 2024
8fbef37
better intellisense for tools/system scripts
pelikhan Aug 13, 2024
1a8f515
format stdout/err in logs
pelikhan Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions docs/genaisrc/genaiscript.d.ts
View file
Open in desktop

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
View file
Open in desktop
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.
Copy link

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
View file
Open in desktop
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.
Copy link

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>
Copy link

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.
Copy link

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")
Copy link

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.
Copy link

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

Copy link

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
View file
Open in desktop

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
View file
Open in desktop
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. There are empty catch blocks which could swallow errors and make debugging difficult.
Copy link

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, "txt")
if (!isQuiet) logVerbose(sres.stdout)
}
if (sres.stderr) {
trace?.detailsFenced(`stderr`, sres.stderr, "txt")
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",
})

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

View workflow job for this annotation

GitHub Actions / build 

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

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
View file
Open in desktop
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,7 +124,7 @@
const ffs = await host.findFiles(arg, {
applyGitIgnore: excludeGitIgnore,
})
if (!ffs.length) {
if (!ffs?.length) {

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

View workflow job for this annotation

GitHub Actions / build 

Potential null reference error. The `ffs` variable is not checked for null or undefined before accessing its `length` property.
Copy link

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
Copy link

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

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