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

feature: refactor prompts into prompt config #29

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

neavra
Copy link

@neavra neavra commented Jan 30, 2025

Pull Request Template

Description

Hi everyone, I'm a first-time contributor here. While working with the Pippin framework, I found that fine-tuning prompts for different activities could get a bit messy. This PR refactors the prompt loading functionality by moving all prompts into a dedicated prompts_config.json file. This centralises the prompt configuration and improves the flexibility of loading different prompts based on activity names.

To facilitate reading the prompts, I've introduced a PromptLoader class that loads the prompts from the configuration file. While I believe this abstraction will make configuring activities easier with minimal changes to existing logic, I'm open to discussing whether this approach is the best way to handle prompts. For now, the DailyThoughtActivity class has been updated to use this new loader.

I think there are a few considerations when abstracting out prompts:

  • Pro: Centralising all prompts in a JSON file allows for easy editing without modifying the code.
  • Pro/Con: The additional dependency in each Activity class introduces some complexity.
  • Pro/Con: Prompts with dynamically loaded variables need to be carefully handled
  • Pro/Con: Onboarding script can be updated to generate the config for the user as well
  • Con: If the JSON file becomes too large, it might affect the PromptLoader's performance when reading the file.

Before I proceed with updating the rest of the activity classes, I'd love feedback on the approach and any suggestions for improvement.

I apologise in advance for any mistakes, as this is my first PR in an open-source project. I'm eager to learn from the community and am open to any guidance or corrections!

Type of Change

  • [O] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Verified that the prompts are correctly loaded from prompts_config.json using the PromptLoader.

Ensured that the DailyThoughtActivity class correctly fetches the prompts based on the activity name.

Tested the prompt retrieval process for different activities to ensure no errors occur when fetching prompts.

Test A: Validate prompt loading from the new prompts_config.json file.

Test B: Ensure no issues occur when running the DailyThoughtActivity with the newly refactored prompt loading process.

Checklist:

  • [O] My code follows the style guidelines of this project
  • [O] I have performed a self-review of my own code
  • [O] I have commented my code, particularly in hard-to-understand areas
  • [O] I have made corresponding changes to the documentation
  • [O] My changes generate no new warnings
  • [O] Any dependent changes have been merged and published in downstream modules

Screenshots (if applicable):

Add any relevant screenshots here.

Additional Notes:

Add any other information that is important to this pull request.

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.

1 participant