Closed pelikhan closed 4 weeks ago
Concerns:
The change in the traceCliArgs
function replaces a detailed YAML configuration example with a link to the CLI test
command. This might reduce the clarity for users who prefer seeing configuration examples directly in the output. ๐ค
The generateCliArguments
function now uses relativePath
from host.projectFolder()
to generate the CLI spec argument. This assumes that host.projectFolder()
will always return a valid project folder path, which might not be the case if the project folder is not properly defined or if the function is called in a context where the project folder is not relevant. ๐ ๏ธ
In the ExtensionState
class, the cliInfo.spec
property is being set with a conditional expression that checks if the filename ends with dir.gpspec.md
and replaces it with **
. This logic seems specific and might not be clear why this replacement is necessary. It could potentially lead to unexpected behavior if the filename convention changes or if there are other .gpspec.md
files that do not follow this pattern. ๐ง
Suggested code improvement:
cliInfo.spec
property, it would be beneficial to add a comment explaining the specific case for dir.gpspec.md
files and the reason for replacing it with **
. This would improve code maintainability and understanding for other developers. ๐// Explain the specific case for 'dir.gpspec.md' files here
spec:
this.host.isVirtualFile(fragment.file.filename) &&
this.host.path.basename(fragment.file.filename) ===
"dir.gpspec.md"
? fragment.file.filename.replace(
/dir\.gpspec\.md$/i,
"**"
)
: // ... rest of the code
Given these concerns, I would suggest addressing them before approving the changes. ๐ง
generated by genaiscript pr-review
Correctly specify glob in file location
CLI Changes
run.md
file to generalize the description of therun
command from "Runs a genai script on a file" to "Runs a script on files".test.mdx
file to document thetest
command, which runs tests in scripts using promptfoo.--models
flag.test view
command to launch the test result viewer.Library Changes
generateCliArguments
function to userelativePath
from thehost
object for generating the CLI command.traceCliArgs
function to provide a link to the CLIrun
command documentation and corrected the text from "The CLI uses" to "The cli uses".traceCliArgs
function by removing the generation of the PromptFoo configuration YAML and updating the text to direct users to the CLItest
command documentation for running script tests.ExtensionState
class to handle virtual file paths differently, including a special case for files nameddir.gpspec.md
.