-
Notifications
You must be signed in to change notification settings - Fork 11
Fix SDK prompt experiments using saved template model #631
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -621,6 +621,50 @@ def test_create_fills_default_prompt_settings_for_prompt_template( | |
| assert call_kwargs["prompt_settings"].temperature == 0.8 | ||
| assert call_kwargs["prompt_settings"].max_tokens == 256 | ||
|
|
||
| @patch("galileo.experiment.create_metric_configs") | ||
| @patch("galileo.experiment.get_prompt") | ||
| @patch("galileo.experiment.load_dataset_and_records") | ||
| @patch("galileo.shared.project_resolver.Projects") | ||
| @patch("galileo.experiment.ExperimentsService") | ||
| def test_create_uses_prompt_template_settings_when_available( | ||
| self, | ||
| mock_experiments_class: MagicMock, | ||
| mock_projects_class: MagicMock, | ||
| mock_load_dataset: MagicMock, | ||
| mock_get_prompt: MagicMock, | ||
| mock_create_metrics: MagicMock, | ||
| reset_configuration: None, | ||
| mock_experiment_response: MagicMock, | ||
| mock_project: MagicMock, | ||
| ) -> None: | ||
| mock_projects_service = MagicMock() | ||
| mock_projects_class.return_value = mock_projects_service | ||
| mock_projects_service.get_with_env_fallbacks.return_value = mock_project | ||
|
|
||
| mock_dataset = MagicMock() | ||
|
Comment on lines
+640
to
+644
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing Given/When/Then in new test
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by Other fix methodsPrompt for AI Agents |
||
| mock_load_dataset.return_value = (mock_dataset, []) | ||
|
|
||
| saved_settings = PromptRunSettings(model_alias="GPT 5.4 Mini (custom)", temperature=0.0, max_tokens=-1) | ||
| mock_prompt = MagicMock() | ||
| mock_prompt.selected_version_id = str(uuid4()) | ||
| mock_prompt.selected_version.settings = saved_settings | ||
| mock_get_prompt.return_value = mock_prompt | ||
|
|
||
| mock_create_metrics.return_value = (None, []) | ||
|
|
||
| mock_experiments_service = MagicMock() | ||
| mock_experiments_class.return_value = mock_experiments_service | ||
| mock_experiments_service.get.return_value = None | ||
| mock_experiments_service.create.return_value = mock_experiment_response | ||
|
|
||
| Experiment( | ||
| name="Test Experiment", dataset_name="test-dataset", prompt_name="test-prompt", project_name="Test Project" | ||
| ).create() | ||
|
|
||
| actual_settings = mock_experiments_service.create.call_args.kwargs["prompt_settings"] | ||
| assert actual_settings is saved_settings | ||
| assert actual_settings.model_alias == "GPT 5.4 Mini (custom)" | ||
|
|
||
| @patch("galileo.experiment.create_metric_configs") | ||
| @patch("galileo.experiment.get_prompt") | ||
| @patch("galileo.experiment.load_dataset_and_records") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,7 +81,10 @@ def experiment_response(): | |
| ) | ||
|
|
||
|
|
||
| def prompt_template(): | ||
| def prompt_template(settings: PromptRunSettings | dict | None = None): | ||
| if settings is None: | ||
| settings = {} | ||
|
|
||
| return PromptTemplate( | ||
| prompt_template=BasePromptTemplateResponse( | ||
| all_available_versions=[1, 2, 3], | ||
|
|
@@ -99,7 +102,7 @@ def prompt_template(): | |
| lines_edited=0, | ||
| lines_removed=0, | ||
| model_changed=False, | ||
| settings={}, | ||
| settings=settings, | ||
| settings_changed=False, | ||
| template="test", | ||
| updated_at=datetime.now(), | ||
|
|
@@ -997,6 +1000,33 @@ def test_run_experiment_w_prompt_template_and_metrics( | |
| # ScorerSettings.create NOT called for trigger=True flow (API handles it) | ||
| mock_scorer_settings_class.return_value.create.assert_not_called() | ||
|
|
||
| @travel(datetime(2012, 1, 1), tick=False) | ||
| @patch.object(galileo.datasets.Datasets, "get") | ||
| @patch.object(galileo.experiments.Experiments, "create", return_value=experiment_response()) | ||
| @patch.object(galileo.experiments.Experiments, "get", return_value=experiment_response()) | ||
| @patch.object(galileo.experiments.Projects, "get_with_env_fallbacks", return_value=project()) | ||
| def test_run_experiment_w_prompt_template_uses_template_settings( | ||
| self, | ||
| mock_get_project: Mock, | ||
| mock_get_experiment: Mock, | ||
| mock_create_experiment: Mock, | ||
| mock_get_dataset: Mock, | ||
| dataset_content: DatasetContent, | ||
| ) -> None: | ||
| saved_settings = PromptRunSettings(model_alias="GPT 5.4 Mini (custom)", temperature=0.0, max_tokens=-1) | ||
|
|
||
| run_experiment( | ||
| "test_experiment", | ||
| project="awesome-new-project", | ||
|
Comment on lines
+1016
to
+1020
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing Given/When/Then in new test
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by Other fix methodsPrompt for AI Agents |
||
| dataset_id=str(UUID(int=0)), | ||
| prompt_template=prompt_template(saved_settings), | ||
| ) | ||
|
|
||
| mock_create_experiment.assert_called_once() | ||
| actual_settings = mock_create_experiment.call_args.kwargs["prompt_settings"] | ||
| assert actual_settings is saved_settings | ||
| assert actual_settings.model_alias == "GPT 5.4 Mini (custom)" | ||
|
|
||
| @travel(datetime(2012, 1, 1), tick=False) | ||
| @patch.object(galileo.datasets.Datasets, "get") | ||
| @patch.object(galileo.experiments.Experiments, "create", return_value=experiment_response()) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_prompt_template_settings()duplicates theselected_version -> settings -> Unsetparsing already done inExperiment.get_prompt_template_settings(), so schema/coercion changes need two edits and the two paths can drift — can we centralize this in one utility reused by both callers? Separately, when template settings are truthy it returnsPromptRunSettings.from_dict(settings)without merging_default_prompt_settings(), so missing required keys stayUNSETand get dropped on serialize whenExperiments.run()orExperiment.create()omitsprompt_settings, silently leaving the job never starting — should we overlay the template dict onto the defaults before returning?Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents