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

simpler markdown table generation #627

Merged
merged 9 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
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
623 changes: 609 additions & 14 deletions THIRD_PARTY_LICENSES.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@
":!genaisrc/*",
":!.github/*",
":!.vscode/*",
":!yarn.lock",
":!*yarn.lock",

Check failure on line 111 in docs/src/content/docs/getting-started/automating-scripts.mdx

View workflow job for this annotation

GitHub Actions / build

The negation pattern for 'yarn.lock' is incorrect, it should not start with an asterisk (*).
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
])

def("GIT_DIFF", changes, {
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/guides/pull-request-reviewer.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
":!genaisrc/*",
":!.github/*",
":!.vscode/*",
":!yarn.lock",
":!*yarn.lock",

Check failure on line 42 in docs/src/content/docs/guides/pull-request-reviewer.mdx

View workflow job for this annotation

GitHub Actions / build

The negation pattern for 'yarn.lock' is incorrect, it should not start with an asterisk (*).
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
])
```

Expand Down
421 changes: 206 additions & 215 deletions docs/yarn.lock

Large diffs are not rendered by default.

19 changes: 9 additions & 10 deletions genaisrc/test-gen.genai.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,9 @@ script({
})

const code = def("CODE", env.files)
const testFiles = env.files.map(({ filename }) => ({ filename: filename.replace(/\.ts$/, ".test.ts") }))
const test = def("TEST", testFiles, { ignoreEmpty: true })

// const { text: keywords } = await runPrompt(_ => {
// const file = _.def("FILE", env.files)
// _.$`You are an expert TypeScript developer.
// Extract the list of exported functions and classes in ${file},
// as a comma separate list of keywords. Be concise.`
// }, { model: "gpt-3.5-turbo" })
// const relevantTests = await retrieval.vectorSearch(keywords, testFiles)
// const tests = def("TESTS", relevantTests)

$`## Step 1

Expand All @@ -22,8 +16,11 @@ generate a plan to test the source code in each file

- use input test files from packages/sample/src/rag/*
- only generate tests for files in ${code}
`

if (testFiles.length) $` - update the existing test files in ${test}. keep old tests if possible.`

## Step 2
$`## Step 2

For each generated test, implement the TypeScript source code in a test file with suffix ".test.ts"
in the same folder as the source file.
Expand All @@ -41,6 +38,7 @@ ${fence('import test, { beforeEach, describe } from "node:test"', { language: "j
- if you need to create files, place them under a "temp" folder
- use Partial<T> to declare a partial type of a type T
- do NOT generate negative test cases
- do NOT generate trace instance

## Step 3

Expand All @@ -60,8 +58,9 @@ defTool(
},
async (args) => {
const { filename, source } = args
if (source)
await workspace.writeText(filename, source)
console.debug(`running test code ${filename}`)
await workspace.writeText(filename, source)
return host.exec(`node`, ["--import", "tsx", "--test", filename])
}
)
2 changes: 1 addition & 1 deletion packages/cli/src/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@
)
trace?.itemValue(`container`, container.id)
trace?.itemValue(`cwd`, cwd)
trace?.item(`\`${command}\` ${args.join(" ")}`)
trace?.fence(`${command} ${args.join(" ")}`, "sh")

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

View workflow job for this annotation

GitHub Actions / build

The method call `trace?.item()` has been changed to `trace?.fence()`. This could potentially cause issues if the `fence` method does not exist or does not have the same functionality as the `item` method. Please ensure that this change is intentional and that the `fence` method is correctly implemented. 🧐
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
if (!isQuiet)
logVerbose(
`container exec: ${command} ${args.join(" ")}`
Expand Down
1 change: 0 additions & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"jsonrepair": "^3.8.0",
"magic-string": "^0.30.11",
"mammoth": "^1.8.0",
"markdown-table": "^3.0.3",
"mathjs": "^13.0.3",
"mime-types": "^2.1.35",
"minimatch": "^10.0.1",
Expand Down
34 changes: 18 additions & 16 deletions packages/core/src/csv.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { parse } from "csv-parse/sync"
import { markdownTable } from "markdown-table"
import { TraceOptions } from "./trace"

export function CSVParse(
Expand Down Expand Up @@ -41,21 +40,24 @@
if (!csv?.length) return ""

Choose a reason for hiding this comment

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

The new implementation of the CSVToMarkdown function does not check if the csv parameter is null before accessing its length property. This can cause a runtime error if the function is called with a null argument.

generated by pr-review-commit missing_null_check


const { headers = Object.keys(csv[0]) } = options || {}
const table: string[][] = [
headers,
...csv.map((row) =>
headers.map((v) => {
const rv = (row as any)[v]
return rv === undefined ? "" : "" + rv
})
const res: string[] = [
`|${headers.join("|")}|`,
`|${headers.map(() => "-").join("|")}|`,
...csv.map(
(row) =>
`|${headers
.map((key) => {
const v = (row as any)[key]
const s = v === undefined || v === null ? "" : "" + v
return s
.replace(/\s+$/, "")
.replace(/[\\`*_{}[\]()#+\-.!]/g, (m) => "\\" + m)
.replace(/</g, "lt;")
.replace(/>/g, "gt;")
.replace(/\r?\n/g, "<br>")
})
.join("|")}|`

Check failure on line 59 in packages/core/src/csv.ts

View workflow job for this annotation

GitHub Actions / build

The CSV parsing logic has been significantly changed. This includes changes to how headers are handled and how each row is processed. This could potentially introduce bugs or change the output format. Please ensure that these changes are thoroughly tested and that the output is as expected. 🧐

Choose a reason for hiding this comment

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

The CSV parsing logic has been significantly changed. This includes changes to how headers are handled and how each row is processed. This could potentially introduce bugs or change the output format. Please ensure that these changes are thoroughly tested and that the new output format is as expected. 🧐

generated by pr-review-commit csv_parsing_change

),
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
]
const md = markdownTable(table, {
stringLength: (str) => str.length,
padding: false,
alignDelimiters: false,
})
// improves LLM performance
const mdcompact = md.replace(/[\t ]+/g, " ")
return mdcompact
return res.join("\n")

Choose a reason for hiding this comment

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

The new implementation of the CSVToMarkdown function does not properly escape the cell content. This can lead to incorrect markdown formatting if the cell content contains markdown special characters.

generated by pr-review-commit missing_escape

Choose a reason for hiding this comment

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

The new implementation of the CSVToMarkdown function does not add a newline at the end of the generated markdown. This can cause issues when appending the generated markdown to other text.

generated by pr-review-commit missing_newline

pelikhan marked this conversation as resolved.
Show resolved Hide resolved
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
}
5 changes: 0 additions & 5 deletions packages/core/src/expander.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,6 @@ export async function expandTemplate(

trace.startDetails("💾 script")

trace.itemValue(`temperature`, temperature)
trace.itemValue(`top_p`, topP)
trace.itemValue(`max tokens`, max_tokens)
trace.itemValue(`seed`, seed)

traceEnv(model, trace, env)
pelikhan marked this conversation as resolved.
Show resolved Hide resolved

trace.startDetails("🧬 prompt")
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/promptdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@
const dtype = language || /\.([^\.]+)$/i.exec(file.filename)?.[1] || ""
let body = file.content
if (/^(c|t)sv$/i.test(dtype)) {
const parsed = CSVTryParse(file.content)
const parsed = !/^\s*|/.test(file.content) && CSVTryParse(file.content)

Check failure on line 164 in packages/core/src/promptdom.ts

View workflow job for this annotation

GitHub Actions / build

A condition has been added to the CSV parsing logic. This could potentially skip parsing for some CSV files that do not meet the condition. Please ensure that this condition is correct and does not unintentionally exclude valid CSV files. 🧐
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
if (parsed) {
body = CSVToMarkdown(parsed)
dfence = ""
Expand Down
4 changes: 2 additions & 2 deletions packages/sample/genaisrc/git-release-notes.genai.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ const { stdout: diff } = await host.exec("git", [
":!docs/**",
":!.github/*",
":!.vscode/*",
":!yarn.lock",
":!THIRD_PARTY_NOTICES.md",
":!*yarn.lock",
":!*THIRD_PARTY_NOTICES.md",
])

const commitsName = def("COMMITS", commits, { maxTokens: 4000 })
Expand Down
7 changes: 4 additions & 3 deletions packages/sample/genaisrc/pr-describe.genai.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ const { stdout: changes } = await host.exec("git", [
"diff",
defaultBranch,
"--",
".",
":!**/genaiscript.d.ts",
":!**/jsconfig.json",
":!**/*sconfig.json",
":!genaisrc/*",
":!.github/*",
":!.vscode/*",
":!yarn.lock",
":!THIRD_PARTY_LICENSES.md",
":!*yarn.lock",
":!*THIRD_PARTY_LICENSES.md",
])

def("GIT_DIFF", changes, {
Expand Down
4 changes: 2 additions & 2 deletions packages/sample/genaisrc/pr-review.genai.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ const { stdout: diff } = await host.exec("git", [
":!genaisrc/*",
":!.github/*",
":!.vscode/*",
":!yarn.lock",
":!THIRD_PARTY_LICENSES.md",
":!*yarn.lock",
":!*THIRD_PARTY_LICENSES.md",
])

def("GIT_DIFF", diff, {
Expand Down
Loading