Skip to content

Support search traces by prompt version#18906

Merged
TomeHirata merged 14 commits intomlflow:masterfrom
TomeHirata:feat/prompt/search-traces
Dec 3, 2025
Merged

Support search traces by prompt version#18906
TomeHirata merged 14 commits intomlflow:masterfrom
TomeHirata:feat/prompt/search-traces

Conversation

@TomeHirata
Copy link
Collaborator

@TomeHirata TomeHirata commented Nov 19, 2025

🛠 DevTools 🛠

Open in GitHub Codespaces

Install mlflow from this PR

# mlflow
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/18906/merge
# mlflow-skinny
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/18906/merge#subdirectory=libs/skinny

For Databricks, use the following command:

%sh curl -LsSf https://raw.githubusercontent.com/mlflow/mlflow/HEAD/dev/install-skinny.sh | sh -s pull/18906/merge

Related Issues/PRs

n/a

What changes are proposed in this pull request?

This PR introduces a new prompts filter for search_traces. As part is this change, we migrate the linkage storage from trace tag to entity association table of tracking store.

traces = mlflow.search_traces(filter_string="prompts = 'prompt-a/1'")

How is this PR tested?

  • Existing unit/integration tests
  • New unit/integration tests
  • Manual tests

Does this PR require documentation update?

  • No. You can skip the rest of this section.
  • Yes. I've updated:
    • Examples
    • API references
    • Instructions

Release Notes

Is this a user-facing change?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release notes for MLflow users.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

  • area/tracking: Tracking Service, tracking client APIs, autologging
  • area/models: MLmodel format, model serialization/deserialization, flavors
  • area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
  • area/scoring: MLflow Model server, model deployment tools, Spark UDFs
  • area/evaluation: MLflow model evaluation features, evaluation metrics, and evaluation workflows
  • area/gateway: MLflow AI Gateway client APIs, server, and third-party integrations
  • area/prompts: MLflow prompt engineering features, prompt templates, and prompt management
  • area/tracing: MLflow Tracing features, tracing APIs, and LLM tracing functionality
  • area/projects: MLproject format, project running backends
  • area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
  • area/build: Build and test infrastructure for MLflow
  • area/docs: MLflow documentation pages

How should the PR be classified in the release notes? Choose one:

  • rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
  • rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
  • rn/feature - A new user-facing feature worth mentioning in the release notes
  • rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
  • rn/documentation - A user-facing documentation change worth mentioning in the release notes

Should this PR be included in the next patch release?

Yes should be selected for bug fixes, documentation updates, and other small changes. No should be selected for new features and larger changes. If you're unsure about the release classification of this PR, leave this unchecked to let the maintainers decide.

What is a minor/patch release?
  • Minor release: a release that increments the second part of the version number (e.g., 1.2.0 -> 1.3.0).
    Bug fixes, doc updates and new features usually go into minor releases.
  • Patch release: a release that increments the third part of the version number (e.g., 1.2.0 -> 1.2.1).
    Bug fixes and doc updates usually go into patch releases.
  • Yes (this PR will be cherry-picked and included in the next patch release)
  • No (this PR will be included in the next minor release)

- Removed the deprecated LINKED_PROMPTS_TAG_KEY from prompt constants.
- Updated all references to linked prompts in the codebase to use TraceTagKey.LINKED_PROMPTS.
- Added validation for prompts filter in search functionality to ensure only exact matches are allowed.
- Enhanced error handling for invalid prompts filter format.

This change centralizes the management of linked prompts and improves the clarity of the codebase.

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
@github-actions github-actions bot added v3.6.1 area/tracing MLflow Tracing and its integrations rn/feature Mention under Features in Changelogs. labels Nov 19, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 19, 2025

Documentation preview for 9d623ce is available at:

Changed Pages (1)

More info
  • Ignore this comment if this PR does not change the documentation.
  • The preview is updated when a new commit is pushed to this PR.
  • This comment was created by this workflow run.
  • The documentation was built by this workflow run.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add some documentation under /genai/prompt-registry/ as well? Maybe it's worth having a single page explains link between prompt and traces. Maybe follow-up after adding UI feature to jump to a list of traces from the prompt page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, let me add the doc section after adding UI changes.


if SearchTraceUtils.is_tag(key_type, comparator):
entity = SqlTraceTag
# Special handling for prompts filter: only support exact match with name/version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we actually use the entity association somehow? Since traces are stored in the experiment, search query goes through tracking store anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could migrate the linkage to EntityAssociation, but what the motivation would be? For storing more structured linkage?

Copy link
Collaborator

@B-Step62 B-Step62 Dec 1, 2025

Choose a reason for hiding this comment

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

For performance. Entity association table stores one row per prompt+trace combination, ooposed to a tag that combines multiple prompts into one row, so we can use the exact match condition and benefit from the index of destination ID (WHERE destination_type = 'prompt' and destination_id = '<name/version>'). This should be much more efficient than LIKE on tag values.

Copy link
Collaborator Author

@TomeHirata TomeHirata Dec 1, 2025

Choose a reason for hiding this comment

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

Understood, do we think the efficiency gain is worth breaking the backward compatibility right now? Current search does not require any data model change, and we can always switch to entity association if users report search slowness.

Copy link
Collaborator

@B-Step62 B-Step62 Dec 1, 2025

Choose a reason for hiding this comment

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

Understood, do we think the efficiency gain is worth breaking the backward compatibility right now?

Yes. Afaik we don't provide any place users can see the link between tag and prompt in OSS side, then I don't think it breaks anything. The migration becomes harder when we do it after adding the search functionality.

If you mean Databricks side, we can keep tag logging for DBX. They also plan to migrate to entity association table fwiw.

Copy link
Collaborator

@B-Step62 B-Step62 Dec 1, 2025

Choose a reason for hiding this comment

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

I think consolidating the linkage in entity association table and populate trace fields (either tag or metadata) from the entity association is clearer since we can keep single source of truth.

Yes, it removes duplication, but one downside is every prompt load needs to make additional SQL query, even if the trace is not linked to any prompt. Since tags are stored in TraceInfo, searching traces also triggers that for every prompt.

Also unfortunately Databricks UI already shows prompts link based on trace tag (which is different from OSS). This means that, even after dbx migrate to entity table, we cannot remove the tag dependence at least for a certain period😔

This means that, the end state will be

Option 1 (yours):

  • Write: Set the tag in dbx only. Store a record in entity association table.
  • Read: Populate tag/metadata at load time.

Option 2:

  • Write: Set the tag and store to entity association table always.
  • Read: none

I'm not sure if the state 1 is cleaner than 2. The single source of truth is nice in general, but not always the best - for example, we can think it as a kind of databse denormalization - we denormalize the info to entity table for search optimization.

Copy link
Collaborator Author

@TomeHirata TomeHirata Dec 2, 2025

Choose a reason for hiding this comment

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

I got your point, but do you think the single additional query for retrieving associations is that problematic? As you said earlier, it should be quick since it relies on database indices, and as I implemented, it does not cause N+1 issue by using batch loading. Btw not necessarily objecting to option 2, we can go with dual writing if skipping one query is really worth database denormalization.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Btw, there are several todo comments to remove tag based linkage, didn't we want to remove the tag based approach?

# TODO: Remove tag based linking once we migrate to LinkPromptsToTraces endpoint

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine with populating it to tags if you feel strongly about it. For the TODO comment, I didn't expect to have "linked prompt" tab in the trace UI but just to have a button like "see linked prompts" that jumps to search page:)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see, shall we defer the decision by implementing option 2 for now? Option 2 -> option 1 is feasible while the opposite is not.

…y across documentation and codebase

- Changed all instances of 'prompts' to 'prompt' in the search_traces API documentation and related code.
- Updated error messages and test cases to reflect the new filter name.
- Ensured that the filter only supports exact matches with the format "name/version".

This change enhances clarity and consistency in the API usage.

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
- Updated the filter validation logic to check for exactly one '/' in the prompt value, improving error handling for invalid inputs.
- Adjusted the test cases to include a new scenario for invalid prompt format.

This change enhances the robustness of the prompts filter functionality.

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
@harupy
Copy link
Member

harupy commented Dec 1, 2025

/review Only review mlflow/store/tracking/sqlalchemy_store.py and suggest a cleaner approach using match statement to validate the filter value.


🚀 Review workflow started


✅ Review completed.

Review Output

Perfect! I've successfully reviewed the PR and added a comment suggesting a cleaner approach using pattern matching.

Summary

I reviewed PR #18906 focusing on mlflow/store/tracking/sqlalchemy_store.py as requested. I found one style improvement opportunity:

Issue Found: The code uses the verbose approach of checking value.count("/") != 1 followed by value.split("/", 1) to validate and parse the prompt filter value format.

Suggestion: Replace this with pattern matching (match/case statement) which is cleaner, more concise, and aligns with MLflow's Python style guide recommendations for string splitting with fixed parts.

The review comment has been posted to the PR with a code suggestion that maintainers can apply with one click: #18906 (comment)

- Introduced the `linkPromptsToTrace` RPC to associate multiple prompt versions with a trace, enhancing prompt tracking workflows.
- Implemented the necessary data structures and request handlers to support this functionality.
- Updated the tracking store interfaces to allow linking prompts to traces using entity associations.
- Added tests to verify the correct behavior of the new API and its integration with the existing system.

This change improves the ability to manage prompt versions in relation to traces, facilitating better tracking and organization of prompt usage.

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
…Store

- Added a method to populate the LINKED_PROMPTS tag from entity associations for traces that lack this tag, ensuring backward compatibility.
- Updated the get_trace_info method to call the new population method.
- Enhanced search_traces to include the LINKED_PROMPTS tag for relevant traces.
- Added unit tests to verify the correct population of LINKED_PROMPTS in various scenarios.

This change improves trace management by maintaining compatibility with existing data structures.

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
…traces

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
@TomeHirata TomeHirata force-pushed the feat/prompt/search-traces branch from 0d368f2 to 643a2a9 Compare December 2, 2025 08:34
Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
@TomeHirata TomeHirata requested a review from B-Step62 December 2, 2025 09:43
Copy link
Collaborator

@B-Step62 B-Step62 left a comment

Choose a reason for hiding this comment

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

LGTM!

@TomeHirata TomeHirata added this pull request to the merge queue Dec 3, 2025
Merged via the queue into mlflow:master with commit 9671b90 Dec 3, 2025
63 of 65 checks passed
@TomeHirata TomeHirata deleted the feat/prompt/search-traces branch December 3, 2025 07:57
BenWilson2 pushed a commit to BenWilson2/mlflow that referenced this pull request Dec 4, 2025
Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
BenWilson2 pushed a commit that referenced this pull request Dec 4, 2025
Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tracing MLflow Tracing and its integrations rn/feature Mention under Features in Changelogs. v3.6.1 v3.7.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants