simpler markdown table generation #627
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // improves LLM performance | ||
| const mdcompact = md.replace(/[\t ]+/g, " ") | ||
| return mdcompact | ||
| return res.join("\n") |
There was a problem hiding this comment.
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
| // improves LLM performance | ||
| const mdcompact = md.replace(/[\t ]+/g, " ") | ||
| return mdcompact | ||
| return res.join("\n") |
There was a problem hiding this comment.
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
| @@ -41,21 +40,22 @@ export function CSVToMarkdown(csv: object[], options?: { headers?: string[] }) { | |||
| if (!csv?.length) return "" | |||
There was a problem hiding this comment.
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
|
/genai-describe |
| .replace(/>/g, "gt;") | ||
| .replace(/\r?\n/g, "<br>") | ||
| }) | ||
| .join("|")}|` |
There was a problem hiding this comment.
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
|
/genai-review |
|
The pull request seems to include several notable changes:
Overall, these changes look good from a functional standpoint. However, for the So, LGTM 🚀.
|
Fix for #618
😀 The
automating-scripts.mdxandpull-request-reviewer.mdxdocuments were slightly modified to change the git exclusion pattern foryarn.lockfiles. These changes only affect the files that these scripts consider.🚀 Trace-related changes were also made, notably, in
docker.ts, a command trace is now fenced instead of being a simple item.🤖 A package called "markdown-table" was removed from
package.json. This suggests that Markdown table handling is now done differently. The functionCSVToMarkdownincsv.tswas re-written and no longer usesmarkdown-table.⚰️ The
csv.test.tsfile was wholly removed. This implies the CSV functionalities have been adequately covered elsewhere in the tests or it is no longer necessary.🧹 In
expander.ts, values relating totemperature,top_p,max tokens, andseedwere removed from tracing. This cuts down on unnecessary logged information.🔧 Several
.jsscripts undergenaisrcwere also tweaked, largely changing the git exclusion pattern foryarn.lockandTHIRD_PARTY_LICENSES.mdfiles.🚩 In
pr-describe.genai.js, an additional argument.was added to the git diff command, this means the diff now considers the current directory more explicitly. Plus, the exclusion pattern for json config files was changed.Please note that none of these changes affects the public API as defined in "packages/core/src/prompt_template.d.ts" and "packages/core/src/prompt_type.ts", therefore, these updates are not user-facing.