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 2 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.

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

Large diffs are not rendered by default.

16 changes: 7 additions & 9 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} 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
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?.item(`${command} ${args.join(" ")}`)

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

View workflow job for this annotation

GitHub Actions / build

The backticks around the command variable have been removed. This could potentially change the output of the trace item if the command contains spaces or special characters. Consider adding them back for consistency and to avoid potential issues. 🤔
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
74 changes: 0 additions & 74 deletions packages/core/src/csv.test.ts
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
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 ""

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("|")}|`

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

),

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

View workflow job for this annotation

GitHub Actions / build

The new implementation of CSVToMarkdown does not properly escape markdown special characters in the cell values. This could lead to incorrect rendering of the markdown table if the cell values contain markdown special characters. Consider adding a function to escape these characters. 😮
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 new implementation of CSVToMarkdown does not add a newline at the end of the generated markdown. This could potentially cause issues when appending or concatenating this markdown with other text or markdown. Consider adding a newline at the end of the generated markdown. 🧐

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
}
Loading