fix(gooddata-eval): delete metrics created during agentic eval runs#1685
Open
myhoai wants to merge 1 commit into
Open
fix(gooddata-eval): delete metrics created during agentic eval runs#1685myhoai wants to merge 1 commit into
myhoai wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAgentic conversations and metric-skill runs now track created metrics and delete them during final cleanup, while preserving metrics for ChangesMetric cleanup
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Sequence Diagram(s)sequenceDiagram
participant MetricSkill
participant ChatClient
participant GoodDataAPI
MetricSkill->>ChatClient: Run metric evaluation
ChatClient->>GoodDataAPI: Create metric
GoodDataAPI-->>ChatClient: Return metric_id
MetricSkill->>GoodDataAPI: Delete workspace metric
GoodDataAPI-->>MetricSkill: Return deletion status
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1685 +/- ##
==========================================
+ Coverage 77.85% 77.95% +0.10%
==========================================
Files 271 271
Lines 18603 18637 +34
==========================================
+ Hits 14483 14529 +46
+ Misses 4120 4108 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py`:
- Around line 130-145: Metric cleanup currently ignores unsuccessful DELETE
responses, allowing stale metrics to remain. In _delete_metric, store the
response from client._client.delete and call raise_for_status() before
returning, while preserving the existing exception logging behavior; follow the
pattern used by alert_skill._delete_alert.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5fc3906-3ce0-41f2-8394-c8f113bb3c9f
📒 Files selected for processing (2)
packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py
myhoai
force-pushed
the
QA-28379-eval-metric-cleanup
branch
from
a6682fc to
0b49776
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py (1)
143-143: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSuppress the intentional broad-catch warning.
Best-effort cleanup justifies catching
Exception, but Ruff still reports BLE001. Add the suppression already proposed in the previous review.- except Exception as exc: + except Exception as exc: # noqa: BLE001 - best-effort cleanup🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py` at line 143, Suppress Ruff’s BLE001 warning for the intentional broad `Exception` handler in the cleanup logic by adding the previously agreed inline suppression to the `except Exception as exc` statement.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py`:
- Around line 162-166: Update the conversation cleanup logic in the metric-skill
evaluation function and its create-metric response handling to collect every
successfully returned metric ID, rather than retaining only the first result.
Preserve the first creation result for evaluation, pass the complete ID
collection to _delete_metric, and ensure all successful create_metric calls are
tracked across responses.
---
Duplicate comments:
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py`:
- Line 143: Suppress Ruff’s BLE001 warning for the intentional broad `Exception`
handler in the cleanup logic by adding the previously agreed inline suppression
to the `except Exception as exc` statement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a104d28-ad34-48b7-a98b-f6be343741f4
📒 Files selected for processing (2)
packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py
Comment on lines
+162
to
+166
| """Drive one full multi-turn metric-skill conversation and evaluate the result. | ||
|
|
||
| Any metric the agent creates during this run is deleted on the way out (see | ||
| ``_delete_metric``) so it cannot leak into — and be reused by — a later test | ||
| sharing the workspace. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Track every successful create_metric call.
Only the first result’s ID is retained. If one response contains multiple successful creation calls, additional metrics remain in the shared workspace. Collect all returned IDs while preserving the first result for evaluation.
Proposed fix
metric_result: dict | None = None
- metric_id_to_delete: str | None = None
+ metric_ids_to_delete: list[str] = []
turns = 0
@@
turns += 1
chat_result = client.send_message(conversation_id, current_question)
- candidate = _extract_metric_result(chat_result.tool_call_events or [])
+ tool_call_events = chat_result.tool_call_events or []
+ candidate = _extract_metric_result(tool_call_events)
+ for tc in tool_call_events:
+ if tc.function_name != "create_metric" or not tc.result:
+ continue
+ result_data = tc.parsed_result()
+ data = result_data.get("data", result_data) if isinstance(result_data, dict) else None
+ metric_id = data.get("metric_id") if isinstance(data, dict) else None
+ if metric_id and metric_id not in metric_ids_to_delete:
+ metric_ids_to_delete.append(metric_id)
+
if candidate is not None:
metric_result = candidate
- metric_id_to_delete = candidate.get("metric_id")
break
@@
finally:
- if metric_id_to_delete:
- _delete_metric(client, workspace_id, metric_id_to_delete)
+ for metric_id in metric_ids_to_delete:
+ _delete_metric(client, workspace_id, metric_id)Also applies to: 170-182, 200-202
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py` around
lines 162 - 166, Update the conversation cleanup logic in the metric-skill
evaluation function and its create-metric response handling to collect every
successfully returned metric ID, rather than retaining only the first result.
Preserve the first creation result for evaluation, pass the complete ID
collection to _delete_metric, and ensure all successful create_metric calls are
tracked across responses.
0b49776 to
e50fad2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/gooddata-eval/tests/test_agentic_conversation.py (1)
174-216: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCover multi-turn and teardown-failure cleanup.
This test covers only one successful turn. Add a two-turn fixture with
$refresolution and duplicate/new metric IDs, plus a failure-path case; assert each unique ID is deleted once even when the conversation exits exceptionally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gooddata-eval/tests/test_agentic_conversation.py` around lines 174 - 216, Expand test_run_agentic_conversation_deletes_created_metrics to use a two-turn fixture with $ref resolution, exercising duplicate and newly created metric IDs and asserting each unique metric is deleted exactly once. Add a separate failure-path test where the conversation raises an exception, then verify teardown still deletes all unique created metric IDs once.packages/gooddata-eval/tests/test_agentic_metric_skill.py (1)
170-201: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd failure-path and multi-run cleanup coverage.
This only exercises one successful metric-skill run. Add coverage for cleanup when evaluation raises and for multiple iterations, asserting that every created metric ID is deleted exactly once.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gooddata-eval/tests/test_agentic_metric_skill.py` around lines 170 - 201, Add tests alongside test_run_agentic_metric_skill_deletes_created_metric covering an evaluation failure that raises after metric creation, asserting the created metric is still deleted exactly once, and covering multiple iterations that create distinct metrics, asserting every returned metric ID is deleted exactly once. Reuse the existing ChatClient mock setup and configure the evaluation inputs or mocked responses to exercise both cleanup paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py`:
- Around line 178-182: Update the metric cleanup logic around
_extract_metric_result and the surrounding chat-result loop to collect every
successful create_metric metric_id, rather than assigning only the first
candidate and breaking. Mirror conversation.run_agentic_conversation’s
aggregation behavior, then ensure all collected IDs are passed to cleanup so no
created metrics leak.
- Line 143: Add an inline <code># noqa: BLE001</code> comment with a brief
rationale to the intentional broad exception handler in the cleanup function
containing <code>except Exception as exc</code>, documenting that cleanup is
best-effort and must not propagate errors.
---
Nitpick comments:
In `@packages/gooddata-eval/tests/test_agentic_conversation.py`:
- Around line 174-216: Expand
test_run_agentic_conversation_deletes_created_metrics to use a two-turn fixture
with $ref resolution, exercising duplicate and newly created metric IDs and
asserting each unique metric is deleted exactly once. Add a separate
failure-path test where the conversation raises an exception, then verify
teardown still deletes all unique created metric IDs once.
In `@packages/gooddata-eval/tests/test_agentic_metric_skill.py`:
- Around line 170-201: Add tests alongside
test_run_agentic_metric_skill_deletes_created_metric covering an evaluation
failure that raises after metric creation, asserting the created metric is still
deleted exactly once, and covering multiple iterations that create distinct
metrics, asserting every returned metric ID is deleted exactly once. Reuse the
existing ChatClient mock setup and configure the evaluation inputs or mocked
responses to exercise both cleanup paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da56ecf8-f8a5-4362-af36-476d65d00ae5
📒 Files selected for processing (4)
packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.pypackages/gooddata-eval/tests/test_agentic_conversation.pypackages/gooddata-eval/tests/test_agentic_metric_skill.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py
| try: | ||
| response = client._client.delete(url, headers=client._auth) | ||
| response.raise_for_status() | ||
| except Exception as exc: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Silence the Ruff BLE001 warning on the intentional best-effort catch.
The blind except Exception is deliberate (cleanup must never propagate), but Ruff still flags it. Add a # noqa: BLE001 with a short rationale to keep the lint clean.
🧹 Proposed fix
- except Exception as exc:
+ except Exception as exc: # noqa: BLE001 - best-effort cleanup, log and continue
print(f"[CLEANUP] Failed to delete metric {metric_id}: {exc}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| except Exception as exc: | |
| except Exception as exc: # noqa: BLE001 - best-effort cleanup, log and continue |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 143-143: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py` at
line 143, Add an inline <code># noqa: BLE001</code> comment with a brief
rationale to the intentional broad exception handler in the cleanup function
containing <code>except Exception as exc</code>, documenting that cleanup is
best-effort and must not propagate errors.
Source: Linters/SAST tools
Comment on lines
+178
to
+182
| candidate = _extract_metric_result(chat_result.tool_call_events or []) | ||
| if candidate is not None: | ||
| metric_result = candidate | ||
| metric_id_to_delete = candidate.get("metric_id") | ||
| break |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only the first created metric is tracked for cleanup.
_extract_metric_result returns just the first create_metric result and the loop breaks immediately, so if a single response contains multiple successful create_metric calls, the extra metrics leak into the shared workspace — the exact failure this PR targets. The sibling conversation.run_agentic_conversation already collects every returned metric_id; consider mirroring that here for consistency.
🐛 Proposed fix
metric_result: dict | None = None
- metric_id_to_delete: str | None = None
+ metric_ids_to_delete: list[str] = []
turns = 0
current_question = question
try:
for _iteration in range(max_iterations):
turns += 1
chat_result = client.send_message(conversation_id, current_question)
- candidate = _extract_metric_result(chat_result.tool_call_events or [])
+ tool_call_events = chat_result.tool_call_events or []
+ candidate = _extract_metric_result(tool_call_events)
+ for tc in tool_call_events:
+ if tc.function_name != "create_metric" or not tc.result:
+ continue
+ result_data = tc.parsed_result()
+ data = result_data.get("data", result_data) if isinstance(result_data, dict) else None
+ metric_id = data.get("metric_id") if isinstance(data, dict) else None
+ if metric_id and metric_id not in metric_ids_to_delete:
+ metric_ids_to_delete.append(metric_id)
if candidate is not None:
metric_result = candidate
- metric_id_to_delete = candidate.get("metric_id")
break
@@
finally:
- if metric_id_to_delete:
- _delete_metric(client, workspace_id, metric_id_to_delete)
+ for metric_id in metric_ids_to_delete:
+ _delete_metric(client, workspace_id, metric_id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py` around
lines 178 - 182, Update the metric cleanup logic around _extract_metric_result
and the surrounding chat-result loop to collect every successful create_metric
metric_id, rather than assigning only the first candidate and breaking. Mirror
conversation.run_agentic_conversation’s aggregation behavior, then ensure all
collected IDs are passed to cleanup so no created metrics leak.
Metric-skill and conversation evaluators create metrics in a shared,
persistent eval workspace but never removed them. A leftover metric is
detected and reused by a later test (the agent returns
`SELECT {existing_id}` instead of full MAQL), so the assertion fails —
and it makes some conversation metric-turns produce no create_metric
call at all.
Delete each metric a run creates on the way out, keyed on the exact
metric_id from the create_metric tool result, in a finally block:
- metric_skill: per-run cleanup in _execute_single_metric_run.
- conversation: collect metric_ids across turns, delete after the
conversation completes (deferred so a later turn's $ref still resolves).
Mirrors the existing alert_skill._delete_alert pattern. Parallelism-safe:
each run deletes only the id it created, so concurrent tests sharing the
workspace never touch each other's objects. No version bump.
JIRA: QA-28379
risk: nonprod
myhoai
force-pushed
the
QA-28379-eval-metric-cleanup
branch
from
e50fad2 to
67ec82b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/gooddata-eval/tests/test_agentic_metric_skill.py (1)
170-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for multi-metric-per-turn cleanup.
This test only exercises a single
create_metricresult per turn, so it doesn't catch the leak described in themetric_skill.pyreview (only the first metric per turn is tracked for deletion). Once that's fixed, add a case with twocreate_metrictool call events in one response and assert both are deleted.def test_run_agentic_metric_skill_deletes_all_created_metrics_in_one_turn(): mock_client = MagicMock() mock_client._base = "http://host/api/v1/ai/workspaces/ws1/chat/conversations" mock_client.create_conversation.return_value = "conv-1" mock_client.send_message.return_value = ChatResult.model_validate( { "textResponse": "done", "toolCallEvents": [ { "functionName": "create_metric", "functionArguments": "{}", "result": '{"data": {"maql": "SELECT {metric/foo}", "metric_id": "foo_metric"}}', }, { "functionName": "create_metric", "functionArguments": "{}", "result": '{"data": {"maql": "SELECT {metric/bar}", "metric_id": "bar_metric"}}', }, ], "reasoningStepCount": 1, } ) with patch("gooddata_eval.core.agentic.metric_skill.ChatClient", return_value=mock_client): run_agentic_metric_skill( host="http://host/api/v1/actions/workspaces/ws1/ai", token="tok", workspace_id="ws1", question="Create metric foo", expected_output={"maql": "SELECT {metric/foo}"}, k=1, max_iterations=1, ) assert mock_client._client.delete.call_count == 2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gooddata-eval/tests/test_agentic_metric_skill.py` around lines 170 - 201, Add a regression test named test_run_agentic_metric_skill_deletes_all_created_metrics_in_one_turn in the existing metric skill test module, configuring ChatResult with two create_metric events containing distinct metric IDs. Run run_agentic_metric_skill with the mocked ChatClient, then assert mock_client._client.delete is called twice and, preferably, verify both metric entity URLs are deleted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py`:
- Around line 130-147: Update _execute_single_metric_run to track all created
metric IDs using _extract_created_metric_ids(tool_call_events), rather than only
candidate.get("metric_id"). Ensure cleanup deletes every returned ID, including
multiple successful create_metric results in one turn, and remove the single-ID
tracking path.
---
Nitpick comments:
In `@packages/gooddata-eval/tests/test_agentic_metric_skill.py`:
- Around line 170-201: Add a regression test named
test_run_agentic_metric_skill_deletes_all_created_metrics_in_one_turn in the
existing metric skill test module, configuring ChatResult with two create_metric
events containing distinct metric IDs. Run run_agentic_metric_skill with the
mocked ChatClient, then assert mock_client._client.delete is called twice and,
preferably, verify both metric entity URLs are deleted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a0368e4-f245-4593-9867-1aa0ae711286
📒 Files selected for processing (4)
packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.pypackages/gooddata-eval/tests/test_agentic_conversation.pypackages/gooddata-eval/tests/test_agentic_metric_skill.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py
Comment on lines
+130
to
+147
| def _extract_created_metric_ids(tool_call_events: list[ToolCallEvent]) -> list[str]: | ||
| """Ids of every metric created by ``create_metric`` calls (a turn may create more than one). | ||
|
|
||
| Used for cleanup so no created metric leaks — unlike ``_extract_metric_result``, which | ||
| returns only the first result for MAQL evaluation. Shared with conversation evaluation. | ||
| """ | ||
| metric_ids: list[str] = [] | ||
| for tc in tool_call_events: | ||
| if tc.function_name != "create_metric" or not tc.result: | ||
| continue | ||
| result_data = tc.parsed_result() | ||
| if not result_data: | ||
| continue | ||
| data = result_data.get("data", result_data) | ||
| metric_id = data.get("metric_id") if isinstance(data, dict) else None | ||
| if metric_id and metric_id not in metric_ids: | ||
| metric_ids.append(metric_id) | ||
| return metric_ids |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only the first metric created per turn is tracked for cleanup — leaks on multi-metric turns.
_execute_single_metric_run still tracks a single metric_id_to_delete from candidate.get("metric_id") instead of using the sibling _extract_created_metric_ids helper (defined right above specifically for this purpose, and already used correctly in conversation.py). If a turn's response contains more than one successful create_metric call, only the one backing metric_result gets deleted; the rest leak into the shared workspace, defeating the cleanup this PR adds and contradicting the function's own docstring claim that "any metric the agent creates during this run is deleted on the way out."
🐛 Proposed fix
primary_expected = expected_outputs[0] if expected_outputs else {}
metric_result: dict | None = None
- metric_id_to_delete: str | None = None
+ metric_ids_to_delete: list[str] = []
turns = 0
current_question = question
try:
for _iteration in range(max_iterations):
turns += 1
chat_result = client.send_message(conversation_id, current_question)
- candidate = _extract_metric_result(chat_result.tool_call_events or [])
+ tool_call_events = chat_result.tool_call_events or []
+ candidate = _extract_metric_result(tool_call_events)
+ for metric_id in _extract_created_metric_ids(tool_call_events):
+ if metric_id not in metric_ids_to_delete:
+ metric_ids_to_delete.append(metric_id)
if candidate is not None:
metric_result = candidate
- metric_id_to_delete = candidate.get("metric_id")
break
response_text = (chat_result.text_response or "").strip()
if _is_asking_clarification(response_text):
current_question = generate_simulated_response(response_text, primary_expected)
else:
break
@@
finally:
- if metric_id_to_delete:
- _delete_metric(client, workspace_id, metric_id_to_delete)
+ for metric_id in metric_ids_to_delete:
+ _delete_metric(client, workspace_id, metric_id)Also applies to: 189-222
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.py` around
lines 130 - 147, Update _execute_single_metric_run to track all created metric
IDs using _extract_created_metric_ids(tool_call_events), rather than only
candidate.get("metric_id"). Ensure cleanup deletes every returned ID, including
multiple successful create_metric results in one turn, and remove the single-ID
tracking path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Metric-skill and conversation evaluators create metrics in a shared, persistent eval workspace but never removed them. A leftover metric is detected and reused by a later test (the agent returns
SELECT {existing_id}instead of full MAQL), so the assertion fails — and it makes some conversation metric-turns produce no create_metric call at all.Delete each metric a run creates on the way out, keyed on the exact metric_id from the create_metric tool result, in a finally block:
Mirrors the existing alert_skill._delete_alert pattern. Parallelism-safe: each run deletes only the id it created, so concurrent tests sharing the workspace never touch each other's objects. No version bump.
JIRA: QA-28379
risk: nonprod
Summary by CodeRabbit