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

Add dry run #892

Merged
merged 6 commits into from
Sep 3, 2024
Merged

Add dry run #892

merged 6 commits into from
Sep 3, 2024

Conversation

kanaqsasak
Copy link
Contributor

Pull Request Description

Summary

This pull request introduces a new --dry-run flag to the CLI application. This flag allows users to see what would be sent to the model without actually sending the request. The changes include updates to the README.md for documentation purposes and modifications to the CLI code to implement the dry-run functionality.

Files Changed

  1. README.md

    • Added a description for the new --dry-run flag under the Application Options section.
  2. cli/cli.go

    • Implemented the logic for handling the --dry-run flag. This includes fetching the pattern and context content (if provided) and displaying the formatted request that would be sent to the model.
  3. cli/flags.go

    • Added the DryRun field to the Flags struct to accommodate the new command-line option.

Code Changes

README.md

 Application Options:
   -u, --url=              Choose ollama url (default: http://127.0.0.1:11434)
   -o, --output=           Output to file
   -n, --latest=           Number of latest patterns to list (default: 0)
+      --dry-run           Show what would be sent to the model without actually sending it

cli/cli.go

 func Cli() (message string, err error) {
 		}
 	}

+	if currentFlags.DryRun {
+		var patternContent string
+		var contextContent string
+
+		if currentFlags.Pattern != "" {
+			pattern, patternErr := fabric.Db.Patterns.GetPattern(currentFlags.Pattern)
+			if patternErr != nil {
+				fmt.Printf("Error getting pattern content: %v\n", patternErr)
+				return "", patternErr
+			}
+			patternContent = pattern.Pattern // Assuming the content is stored in the 'Pattern' field
+		}
+
+		if currentFlags.Context != "" {
+			context, contextErr := fabric.Db.Contexts.GetContext(currentFlags.Context)
+			if contextErr != nil {
+				fmt.Printf("Error getting context content: %v\n", contextErr)
+				return "", contextErr
+			}
+			contextContent = context.Content
+		}
+
+		systemMessage := strings.TrimSpace(contextContent) + strings.TrimSpace(patternContent)
+		userMessage := strings.TrimSpace(currentFlags.Message)
+
+		fmt.Println("Dry run: Would send the following request:\n")
+		if systemMessage != "" {
+			fmt.Printf("System:\n%s\n\n", systemMessage)
+		}
+		if userMessage != "" {
+			fmt.Printf("User:\n%s\n", userMessage)
+		}
+		return "", nil
+	}

cli/flags.go

 type Flags struct {
 	YouTube                 string  `short:"y" long:"youtube" description:"YouTube video url to grab transcript, comments from it and send to chat"`
 	YouTubeTranscript       bool    `long:"transcript" description:"Grab transcript from YouTube video and send to chat"`
 	YouTubeComments         bool    `long:"comments" description:"Grab comments from YouTube video and send to chat"`
+	DryRun                  bool    `long:"dry-run" description:"Show what would be sent to the model without actually sending it"`
 }

Reason for Changes

The --dry-run flag is introduced to provide users with a way to preview the request that would be sent to the model. This can be useful for debugging and ensuring that the correct data is being sent without making an actual request.

Impact of Changes

  • Functionality: Adds a new feature allowing users to perform a dry run of their request.
  • Usability: Improves the user experience by providing a way to verify requests before sending them.
  • Codebase: Introduces additional conditional logic to handle the --dry-run flag.

Test Plan

  • Manually test the --dry-run flag with various combinations of patterns, contexts, and messages to ensure the correct output is displayed.
  • Verify that the application behaves as expected when the --dry-run flag is not used.

Additional Notes

  • Ensure that any existing automated tests that may interact with the CLI are updated to account for the new --dry-run functionality.
  • Consider adding automated tests specifically for the --dry-run feature in future updates.

@eugeis
Copy link
Collaborator

eugeis commented Aug 29, 2024

Thank you for the PR.

Please refactor and integrate it to the Chatter struct:

...
var chatter *core.Chatter
if chatter, err = fabric.GetChatter(currentFlags.Model, currentFlags.Stream, currentFlags.DryRun); err != nil {
return
}
....

type Chatter struct {
db *db.Db

Stream bool
DryRun bool

model  string
vendor vendors.Vendor

}

Please implement a DryRun Vendor and the builder method fabric.GetChatter(..., dryRun) shall return the chatter instance with the DrynRunVendor in the field Chatter.vendor, so we don't need to adapt which prints prompts without sending it.

@kanaqsasak
Copy link
Contributor Author

@eugeis I've refactored the dry run to a DryRun vendor as requested and ensured that the dry run model is excluded when listing models.

Let me know if there's anything else you'd like to adjust.

Copy link
Collaborator

@eugeis eugeis left a comment

Choose a reason for hiding this comment

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

Great, please apply those two comments and we would be ready to merge.

core/fabric.go Outdated Show resolved Hide resolved
core/models.go Outdated Show resolved Hide resolved
@eugeis eugeis merged commit a921b77 into danielmiessler:main Sep 3, 2024
1 check passed
eugeis added a commit that referenced this pull request Oct 19, 2024
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.

2 participants