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 4 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 

Invalid negation pattern in gitignore-style exclusions, should be `:!yarn.lock` instead of `:*yarn.lock`.
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 

Invalid negation pattern in gitignore-style exclusions, should be `:!yarn.lock` instead of `:*yarn.lock`.
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 on the `trace` object has been changed from `item` to `fence`. This could potentially cause issues if the `fence` method does not exist or does not behave as expected. 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
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
74 changes: 0 additions & 74 deletions packages/core/src/csv.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,74 +0,0 @@
import { describe, test, beforeEach } from "node:test";
import assert from "node:assert/strict";
import { CSVParse, CSVTryParse, CSVToMarkdown } from "./csv";

describe('CSVParse', () => {
test('Parse simple CSV data with default options', () => {
const csv = "name,age\nJohn,30\nJane,25";
const result = CSVParse(csv);
assert.deepEqual(result, [
{ name: "John", age: "30" },
{ name: "Jane", age: "25" }
]);
});

test('Parse CSV data with custom delimiter', () => {
const csv = "name|age\nJohn|30\nJane|25";
const result = CSVParse(csv, { delimiter: "|" });
assert.deepEqual(result, [
{ name: "John", age: "30" },
{ name: "Jane", age: "25" }
]);
});

test('Parse CSV data with specified headers', () => {
const csv = "John,30\nJane,25";
const result = CSVParse(csv, { headers: ["name", "age"] });
assert.deepEqual(result, [
{ name: "John", age: "30" },
{ name: "Jane", age: "25" }
]);
});
});

describe('CSVTryParse', () => {
test('Try to parse valid CSV data', () => {
const csv = "name,age\nJohn,30\nJane,25";
const result = CSVTryParse(csv);
assert.deepEqual(result, [
{ name: "John", age: "30" },
{ name: "Jane", age: "25" }
]);
});
});

describe('CSVToMarkdown', () => {
test('Convert parsed CSV data to markdown table', () => {
const csv = [{ name: "John", age: "30" }, { name: "Jane", age: "25" }];
const result = CSVToMarkdown(csv);
const expected = `
|name|age|
|-|-|
|John|30|
|Jane|25|
`.trim().replace(/[\t ]+/g, " ");
assert.equal(result, expected);
});

test('Convert parsed CSV data to markdown table with custom headers', () => {
const csv = [{ name: "John", age: "30" }, { name: "Jane", age: "25" }];
const result = CSVToMarkdown(csv, { headers: ["age", "name"] });
const expected = `
|age|name|
|-|-|
|30|John|
|25|Jane|
`.trim().replace(/[\t ]+/g, " ");
assert.equal(result, expected);
});

test('Handle empty CSV data input', () => {
const result = CSVToMarkdown([]);
assert.equal(result, "");
});
});
32 changes: 16 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 @@ -40,22 +39,23 @@
export function CSVToMarkdown(csv: object[], options?: { headers?: string[] }) {
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(/\r?\n/g, "<br>")
})
.join("|")}|`
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")

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

View workflow job for this annotation

GitHub Actions / build 

The implementation of the `CSVToMarkdown` function has been significantly changed. The new implementation manually constructs the markdown table instead of using the `markdownTable` function from the `markdown-table` library. This could potentially introduce bugs or inconsistencies in the output. Please ensure that this change is intentional and that the new implementation correctly generates markdown tables.
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,12 +197,7 @@

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)

Check failure on line 200 in packages/core/src/expander.ts

View workflow job for this annotation

GitHub Actions / build 

Several `trace.itemValue` calls have been removed. These calls are typically used for logging or debugging purposes. Their removal could make it harder to debug issues or understand the program's behavior. Please ensure that this change is intentional and that it does not negatively impact the program's maintainability.
pelikhan marked this conversation as resolved.
Show resolved Hide resolved

trace.startDetails("🧬 prompt")
trace.detailsFenced("📓 script source", template.jsSource, "js")
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