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

write trace to file as it comes #635

Merged
merged 2 commits into from
Aug 21, 2024
Merged
Changes from all commits
Commits
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
41 changes: 33 additions & 8 deletions packages/cli/src/run.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { capitalize } from "inflection"
import { resolve, join, relative } from "node:path"
import { resolve, join, relative, dirname } from "node:path"
import { isQuiet } from "./log"
import { emptyDir, ensureDir } from "fs-extra"
import { convertDiagnosticsToSARIF } from "./sarif"
Expand All @@ -24,6 +24,7 @@
CLI_RUN_FILES_FOLDER,
ANNOTATION_ERROR_CODE,
GENAI_ANY_REGEX,
TRACE_CHUNK,
} from "../../core/src/constants"
import { isCancelError, errorMessage } from "../../core/src/error"
import { Fragment, GenerationResult } from "../../core/src/generation"
Expand All @@ -36,7 +37,11 @@
JSONSchemaStringifyToTypeScript,
JSONSchemaStringify,
} from "../../core/src/schema"
import { TraceOptions, MarkdownTrace } from "../../core/src/trace"
import {
TraceOptions,
MarkdownTrace,
TraceChunkEvent,
} from "../../core/src/trace"
import {
normalizeFloat,
normalizeInt,
Expand All @@ -51,8 +56,21 @@
azureDevOpsParseEnv,
azureDevOpsUpdatePullRequestDescription,
} from "../../core/src/azuredevops"
import { estimateTokens } from "../../core/src/tokens"
import { resolveTokenEncoder } from "../../core/src/encoders"
import { appendFile, writeFile } from "fs/promises"

async function setupTraceWriting(trace: MarkdownTrace, filename: string) {
await ensureDir(dirname(filename))
await writeFile(filename, "", { encoding: "utf-8" })
trace.addEventListener(
TRACE_CHUNK,
async (ev) => {
const tev = ev as TraceChunkEvent
await appendFile(filename, tev.chunk, { encoding: "utf-8" })
},
false
)

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

View workflow job for this annotation

GitHub Actions / build

Using an async function as an event listener can lead to unhandled promise rejections if the promise is rejected. Consider wrapping the async function call within the event listener in a try-catch block to handle any potential errors. πŸ› οΈ

Choose a reason for hiding this comment

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

Using an async function as an event listener can lead to unhandled promise rejections if the promise is rejected. Consider wrapping the async function call within the event listener in a try-catch block to handle any potential errors. πŸ› οΈ

generated by pr-review-commit async_in_event_listener

}

export async function runScriptWithExitCode(
scriptId: string,
Expand Down Expand Up @@ -113,6 +131,18 @@
return { exitCode, result }
}

if (out) {
if (removeOut) await emptyDir(out)
await ensureDir(out)
}
if (outTrace && trace) await setupTraceWriting(trace, outTrace)
if (out && trace) {
const ofn = join(out, "res.trace.md")
if (ofn !== outTrace) {
await setupTraceWriting(trace, ofn)
}
}
pelikhan marked this conversation as resolved.
Show resolved Hide resolved

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

View workflow job for this annotation

GitHub Actions / build

The setup for trace writing is only done if 'out' and 'trace' are truthy. However, 'outTrace' is used later in the code without checking if it was set up correctly. This could potentially lead to errors if 'outTrace' was not set up because 'out' or 'trace' were falsy. Please ensure that 'outTrace' is always correctly set up. 🧐

Choose a reason for hiding this comment

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

The setup for trace writing is only done if 'out' and 'trace' are truthy. However, 'outTrace' is used later in the code without checking if it was set up correctly. This could potentially lead to errors if 'outTrace' was not set up because 'out' or 'trace' were falsy. Please ensure that 'outTrace' is always correctly set up. 🧐

generated by pr-review-commit conditional_trace_setup

const toolFiles: string[] = []
const resolvedFiles = new Set<string>()

Expand Down Expand Up @@ -233,8 +263,7 @@
if (!isQuiet) logVerbose("") // force new line
if (result.status !== "success")
logVerbose(result.statusText ?? result.status)

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

View workflow job for this annotation

GitHub Actions / build

The line of code that writes the trace content to 'outTrace' has been removed. If 'outTrace' is expected to contain the trace content, this could lead to unexpected behavior. Please ensure that the trace content is written to 'outTrace' as expected. πŸ•΅οΈβ€β™€οΈ

Choose a reason for hiding this comment

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

The line of code that writes the trace content to 'outTrace' has been removed. If 'outTrace' is expected to contain the trace content, this could lead to unexpected behavior. Please ensure that the trace content is written to 'outTrace' as expected. πŸ•΅οΈβ€β™€οΈ

generated by pr-review-commit removed_trace_write

if (outTrace) await writeText(outTrace, trace.content)
if (outAnnotations && result.annotations?.length) {
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
if (isJSONLFilename(outAnnotations))
await appendJSONL(outAnnotations, result.annotations)
Expand Down Expand Up @@ -263,8 +292,6 @@
? JSON.stringify(result.messages, null, 2)
: undefined
if (out) {
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
if (removeOut) await emptyDir(out)
await ensureDir(out)
const jsonf = join(out, `res.json`)
const yamlf = join(out, `res.yaml`)

Expand All @@ -273,7 +300,6 @@
const outputf = mkfn(".output.md")
const outputjson = mkfn(".output.json")
const outputyaml = mkfn(".output.yaml")
const tracef = mkfn(".trace.md")
const annotationf = result.annotations?.length
? mkfn(".annotations.csv")
: undefined
Expand All @@ -289,7 +315,6 @@
await writeText(outputyaml, YAMLStringify(result.json))
}
if (result.text) await writeText(outputf, result.text)
if (trace) await writeText(tracef, trace.content)
if (result.schemas) {
for (const [sname, schema] of Object.entries(result.schemas)) {
await writeText(
Expand Down