Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions sdks/python/src/opik/evaluation/evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ def _validated_task(data: Dict[str, Any]) -> Any:
experiment_url = url_helpers.get_experiment_url_by_id(
experiment_id=experiment_.id,
dataset_id=suite_dataset.id,
url_override=client.config.url_override,
base_url=client.config.url_override,
workspace=client._workspace,
)
report.display_evaluation_in_progress(experiment_url)

Expand Down Expand Up @@ -604,7 +605,8 @@ def _evaluate_task(
experiment_url = url_helpers.get_experiment_url_by_id(
experiment_id=experiment.id,
dataset_id=dataset.id,
url_override=client.config.url_override,
base_url=client.config.url_override,
workspace=client._workspace,
Comment on lines 605 to +609

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Placeholder workspace builds wrong URLs

_evaluate_task() and the other evaluator link sites pass workspace=client._workspace into url_helpers.get_experiment_url_by_id(), so when OpikConfig.workspace is the "default" placeholder the compare link points at /opik/default/... instead of the resolved workspace — should we route this through Opik._dereferenced_workspace() first?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
sdks/python/src/opik/evaluation/evaluator.py around lines 549-630 inside
`_evaluate_task`, the `experiment_url = url_helpers.get_experiment_url_by_id(...)` call
passes `workspace=client._workspace`, which embeds the raw workspace placeholder into
the URL. Refactor it to pass the dereferenced/resolved workspace (e.g.,
`workspace=client._dereferenced_workspace()` or an equivalent public accessor) so when
the configured workspace is "default" the generated link targets the actual workspace.
Apply the same fix to the other `url_helpers.get_experiment_url_by_id()` occurrences in
this file: `__internal_api__run_test_suite__` (~lines 408-415), `evaluate_prompt`
(~1192-1199), `evaluate_experiment` (~886-892), and `_evaluate_test_suite_task`
(~739-745).

)

report.display_experiment_link(experiment_url=experiment_url)
Expand Down Expand Up @@ -737,7 +739,8 @@ def _evaluate_test_suite_task(
experiment_url = url_helpers.get_experiment_url_by_id(
experiment_id=experiment.id,
dataset_id=dataset.id,
url_override=client.config.url_override,
base_url=client.config.url_override,
workspace=client._workspace,
)

evaluation_result_ = evaluation_result.EvaluationResult(
Expand Down Expand Up @@ -883,7 +886,8 @@ def evaluate_experiment(
experiment_url = url_helpers.get_experiment_url_by_id(
experiment_id=experiment.id,
dataset_id=dataset_.id,
url_override=client.config.url_override,
base_url=client.config.url_override,
workspace=client._workspace,
)

report.display_experiment_link(experiment_url=experiment_url)
Expand Down Expand Up @@ -1188,7 +1192,8 @@ def evaluate_prompt(
experiment_url = url_helpers.get_experiment_url_by_id(
experiment_id=experiment.id,
dataset_id=dataset.id,
url_override=client.config.url_override,
base_url=client.config.url_override,
workspace=client._workspace,
)

report.display_experiment_link(experiment_url=experiment_url)
Expand Down
13 changes: 5 additions & 8 deletions sdks/python/src/opik/url_helpers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import base64
import json
import urllib.parse
from typing import Final

Expand Down Expand Up @@ -28,15 +29,11 @@ def get_ui_url() -> str:


def get_experiment_url_by_id(
dataset_id: str, experiment_id: str, url_override: str
dataset_id: str, experiment_id: str, base_url: str, workspace: str
) -> str:
encoded_opik_url = base64.b64encode(url_override.encode("utf-8")).decode("utf-8")

project_path = urllib.parse.quote(
f"v1/session/redirect/experiments/?experiment_id={experiment_id}&dataset_id={dataset_id}&path={encoded_opik_url}",
safe=ALLOWED_URL_CHARACTERS,
)
return urllib.parse.urljoin(ensure_ending_slash(url_override), project_path)
domain_root = get_base_url(base_url)
experiments_param = urllib.parse.quote(json.dumps([experiment_id]))
return f"{domain_root}opik/{workspace}/experiments/{dataset_id}/compare?experiments={experiments_param}"
Comment on lines 31 to +36

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Undocumented public URL contract

get_experiment_url_by_id() now uses workspace to build the compare-page link, but the public contract isn't documented, so callers can't tell that workspace is required or what URL shape they get — can we add a short docstring covering that?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
sdks/python/src/opik/url_helpers.py around lines 31-36, update the public function
`get_experiment_url_by_id` (new signature with `dataset_id`, `experiment_id`,
`base_url`, `workspace`) to include a short docstring. Explain that `workspace` is
required to scope the returned link and that the function returns the direct experiments
compare page URL in the form
`...opik/{workspace}/experiments/{dataset_id}/compare?experiments=...`. Keep it
consistent with the surrounding URL helper documentation so callers can rely on the
documented contract.



def get_project_url_by_workspace(
Expand Down
44 changes: 44 additions & 0 deletions sdks/python/tests/unit/test_url_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,47 @@ def test_get_test_suite_url_by_id__returns_direct_project_scoped_url(
)
== expected_test_suite_url
)


@pytest.mark.parametrize(
("base_url", "workspace", "dataset_id", "experiment_id", "expected_url"),
[
(
"http://localhost:5173/api",
"default",
"dataset-abc123",
"exp-xyz789",
"http://localhost:5173/opik/default/experiments/dataset-abc123/compare?experiments=%5B%22exp-xyz789%22%5D",
),
(
"http://localhost:5173/api/",
"default",
"dataset-abc123",
"exp-xyz789",
"http://localhost:5173/opik/default/experiments/dataset-abc123/compare?experiments=%5B%22exp-xyz789%22%5D",
),
(
"https://www.comet.com/opik/api",
"my-team",
"ds-id-001",
"exp-id-002",
"https://www.comet.com/opik/my-team/experiments/ds-id-001/compare?experiments=%5B%22exp-id-002%22%5D",
),
],
)
def test_get_experiment_url_by_id__returns_direct_workspace_scoped_url(
base_url: str,
workspace: str,
dataset_id: str,
experiment_id: str,
expected_url: str,
) -> None:
assert (
url_helpers.get_experiment_url_by_id(
base_url=base_url,
workspace=workspace,
dataset_id=dataset_id,
experiment_id=experiment_id,
)
== expected_url
)
Loading