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

simpler markdown table generation #627

Merged
merged 9 commits into from
Aug 19, 2024
Merged

simpler markdown table generation #627

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6e47e2f
simpler markdown table generation
pelikhan Aug 19, 2024
d1cc290
logging tweak
pelikhan Aug 19, 2024
4bceb29
Refactor gitignore patterns for yarn.lock file
pelikhan Aug 19, 2024
012e5c9
specify fnece language
pelikhan Aug 19, 2024
b4a83c8
more escaping
pelikhan Aug 19, 2024
11084d9
don't double escape markdown tables
pelikhan Aug 19, 2024
b85c730
recovered tests
pelikhan Aug 19, 2024
7f1ad4a
shorter headers
pelikhan Aug 19, 2024
e94b9ec
udpated testgen
pelikhan Aug 19, 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
623 changes: 609 additions & 14 deletions THIRD_PARTY_LICENSES.md
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/src/content/docs/getting-started/automating-scripts.mdx
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

Large diffs are not rendered by default.

16 changes: 7 additions & 9 deletions genaisrc/test-gen.genai.mjs
View file
Open in desktop
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} using a diff format.`

## 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 Down
2 changes: 1 addition & 1 deletion packages/cli/src/docker.ts
View file
Open in desktop
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 has a different behavior or side effects compared to the `item` method. Please ensure that this change is intentional and that the `fence` method is suitable for this use case. 🧐
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
View file
Open in desktop
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
View file
Open in desktop
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 ""
Copy link

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 new output format is as expected. 🧐
Copy link

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

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

Copy link

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
View file
Open in desktop
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
View file
Open in desktop
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 inputs that were previously parsed. Please ensure that this change is intentional and that it does not unintentionally exclude valid inputs. 🧐
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading