Add full custom query for observations#39
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Supports JOINs, CTEs, subqueries, and complex aggregations. " | ||
| "Modifying operations (INSERT, UPDATE, DELETE) are not allowed.", | ||
| ) | ||
| full_custom_query = st.text_area( | ||
| "Full Custom Query", value="", key="full_custom_query_input_key" | ||
| ) |
There was a problem hiding this comment.
The new form field for full_custom_query is always initialised with an empty string (st.text_area(..., value="", ...)) while the submission code later passes that value to the update handler. When an observation that already has a custom query is opened for editing, the text area shows up blank and submitting the form (even without touching that field) overwrites the existing SQL with an empty string. After any edit the observation silently reverts to template-based rendering. Pre-populate the field with the existing value, similar to custom_test_ids_query, so that updates don’t erase user-provided custom queries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| full_custom_query = st.text_area( | ||
| "Full Custom Query", value="", key="full_custom_query_input_key" | ||
| ) |
There was a problem hiding this comment.
The new text area for full_custom_query is always initialized with an empty string. When a user edits an existing observation the saved query is not shown, and asdict later sends the empty value back to update_observation, wiping the stored SQL even if the user never touched the field. This causes data loss whenever any other attribute is changed. Prefilling the widget with predefined.full_custom_query (defaulting to None) would keep existing queries intact.
Useful? React with 👍 / 👎.
| full_custom_query = st.text_area( | ||
| "Full Custom Query", value="", key="full_custom_query_input_key" | ||
| ) |
There was a problem hiding this comment.
full_custom_query is taken straight from the Streamlit form and executed by the runner, but unlike custom_test_ids_query it is never passed through ValidationUtils.validate_sql_query. A user can therefore submit mutating statements (INSERT, DROP, etc.) that will execute under application credentials. Because other SQL fields are explicitly validated to be read-only, this omission is a security regression. The field should be validated or otherwise constrained before rendering/execution.
Useful? React with 👍 / 👎.
| comment="Custom query to calculate the test_ids for the experiment. " | ||
| "The query will be used in jinja template instead of the default query", | ||
| ) | ||
| full_custom_query = Column( | ||
| Text, | ||
| nullable=True, | ||
| comment="Full custom query to calculate metrics. " | ||
| "If provided, this query will be used instead of the template-based query.", | ||
| ) |
There was a problem hiding this comment.
The ORM model now defines a full_custom_query column, but no schema migration accompanies the change. Existing installations will have an observations table without this column, so attempts to create or update an observation that includes full_custom_query will fail with database errors (e.g. “no such column: observations.full_custom_query”). A migration or other schema update is required to keep the database consistent with the model.
Useful? React with 👍 / 👎.
1 participant
This commit introduces the ability to run experiment observations using a fully custom SQL query, bypassing the standard template-based query generation. This provides greater flexibility for complex or non-standard analyses.
closed #29