Skip to content

Support loading prompt from yml file#44

Merged
sgoedecke merged 1 commit intomainfrom
sgoedecke/support-loading-prompt-from-yml-file
May 8, 2025
Merged

Support loading prompt from yml file#44
sgoedecke merged 1 commit intomainfrom
sgoedecke/support-loading-prompt-from-yml-file

Conversation

@sgoedecke
Copy link
Copy Markdown
Collaborator

Supports loading prompt from a yml file via the --file parameter.

Copilot AI review requested due to automatic review settings May 8, 2025 01:43
@sgoedecke sgoedecke requested a review from a team as a code owner May 8, 2025 01:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds the ability to load a prompt definition from a YAML file via the --file flag, preload its messages and parameters, and include a test verifying basic YAML loading.

  • Introduce s.prompt.yml as an example prompt definition.
  • Parse .prompt.yml into a promptFile struct in run.go and inject its data before executing the command.
  • Add a test in run_test.go to confirm that YAML-loaded messages and temperature are applied.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
s.prompt.yml New example YAML file defining prompt metadata.
cmd/run/run.go Parse --file flag, unmarshal YAML, preload messages and parameters.
cmd/run/run_test.go Test that --file loads YAML and applies its settings.
Comments suppressed due to low confidence (1)

cmd/run/run_test.go:85

  • There is no test verifying that a CLI flag overrides a value from the YAML file. Add a case where you pass --temperature alongside --file and assert that the flag value takes effect.
t.Run("--file pre-loads YAML and CLI overrides take precedence", func(t *testing.T) {

Comment thread cmd/run/run.go
@sgoedecke sgoedecke force-pushed the sgoedecke/support-loading-prompt-from-yml-file branch from b5d7883 to f74c60a Compare May 8, 2025 02:03
@sgoedecke sgoedecke merged commit 6d289c6 into main May 8, 2025
5 checks passed
@sgoedecke sgoedecke deleted the sgoedecke/support-loading-prompt-from-yml-file branch May 8, 2025 02:12
Comment thread cmd/run/run.go
Comment thread cmd/run/run.go
Comment thread cmd/run/run.go
Example: "gh models run openai/gpt-4o-mini \"how many types of hyena are there?\"",
Args: cobra.ArbitraryArgs,
RunE: func(cmd *cobra.Command, args []string) error {
filePath, _ := cmd.Flags().GetString("file")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That _, is that an error? Should you maybe check it and return early if it's non-nil?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's probably wise to do that, but I think it's safe to follow up in a later change. The possible errors here are things like "you defined the file flag as an integer but are trying to GetString it", not really things that users could trigger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants