feat(gooddata-eval): optional run_metadata_extra on agentic evaluators#1686
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAgentic evaluation functions now accept optional extra run metadata, forward it to ChangesLangfuse metadata propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Evaluator
participant build_run_context
participant Langfuse
Evaluator->>build_run_context: Forward run_metadata_extra
build_run_context->>build_run_context: Merge metadata and set resolved model_version
build_run_context->>Langfuse: Create dataset-run context
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1686 +/- ##
==========================================
+ Coverage 77.85% 77.91% +0.05%
==========================================
Files 271 271
Lines 18603 18603
==========================================
+ Hits 14483 14494 +11
+ Misses 4120 4109 -11 ☔ 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.
🧹 Nitpick comments (1)
packages/gooddata-eval/src/gooddata_eval/core/agentic/_langfuse.py (1)
398-402: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a unit test for the merge/override behavior.
run_metadata_extracan silently overwrite reserved keys (testing_framework,model_version) sincemetadata.update(run_metadata_extra)runs unconditionally after they're set. This is likely intentional (caller override), but a test asserting both the default-preserving path and the override path would guard against regressions, especially since this metadata is used for report isolation (e.g.github_run_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/_langfuse.py` around lines 398 - 402, Add unit tests for the metadata construction logic in the relevant agentic evaluation function, covering defaults when no extra metadata is supplied and confirming that run_metadata_extra intentionally overrides testing_framework and model_version while preserving additional keys such as github_run_id.
🤖 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.
Nitpick comments:
In `@packages/gooddata-eval/src/gooddata_eval/core/agentic/_langfuse.py`:
- Around line 398-402: Add unit tests for the metadata construction logic in the
relevant agentic evaluation function, covering defaults when no extra metadata
is supplied and confirming that run_metadata_extra intentionally overrides
testing_framework and model_version while preserving additional keys such as
github_run_id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b678bad-788a-4ac6-9cf7-5a6a7e511f22
📒 Files selected for processing (8)
packages/gooddata-eval/src/gooddata_eval/core/agentic/_langfuse.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/alert_skill.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/general_question.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/guardrail.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/metric_skill.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/search_tool.pypackages/gooddata-eval/src/gooddata_eval/core/agentic/visualization.py
myhoai
force-pushed
the
qa-28482-run-metadata-extra
branch
from
252207e to
d1f9189
Compare
hkad98
reviewed
Jul 13, 2026
Comment on lines
+388
to
+394
| """Return (run_name_base, run_metadata) with model version resolved from workspace API. | ||
|
|
||
| :param run_metadata_extra: optional extra key/values added to the dataset-run | ||
| metadata (e.g. a CI run id for scoping). Default None keeps behavior unchanged. | ||
| SDK-owned keys ("testing_framework", "model_version") always take precedence and | ||
| cannot be overwritten by this dict. | ||
| """ |
Contributor
There was a problem hiding this comment.
Python SDK uses Google style docstring. Please change it.
hkad98
reviewed
Jul 13, 2026
| metadata: dict[str, Any] = {"testing_framework": "tavern-e2e"} | ||
| # Start from caller extras, then set SDK-owned keys last so they always win. | ||
| metadata: dict[str, Any] = dict(run_metadata_extra) if run_metadata_extra else {} | ||
| metadata["testing_framework"] = "tavern-e2e" |
Contributor
There was a problem hiding this comment.
Do not hardcode this in this repository.
Add an optional run_metadata_extra parameter to build_run_context and every evaluate_agentic_* entrypoint. Its keys are added to the Langfuse dataset-run metadata; default None keeps behavior unchanged for existing callers. The SDK-derived model_version is applied last and cannot be overwritten by it. Stop hardcoding the consumer-specific "tavern-e2e" testing_framework tag in the SDK (both build_run_context and the observe() fallback); callers now supply their own run tags via run_metadata_extra. This enables a caller (e.g. gdc-nas tavern-e2e CI) to stamp testing_framework + a github_run_id for exact report isolation, without the SDK reading any ambient env itself. Docstring converted to Google style. Includes unit coverage for the merge, model_version precedence, and default paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
myhoai
force-pushed
the
qa-28482-run-metadata-extra
branch
from
d1f9189 to
d599be3
Compare
hkad98
approved these changes
Jul 13, 2026
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
2 participants
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.
Add an optional run_metadata_extra parameter to build_run_context and every evaluate_agentic_* entrypoint. When provided, its keys are merged into the Langfuse dataset-run metadata; default None keeps behavior byte-for-byte identical, so existing callers are unaffected.
Enables a caller (e.g. gdc-nas tavern-e2e CI) to stamp a github_run_id on runs for exact report isolation, without the SDK reading any ambient env itself.
Summary by CodeRabbit
run_metadata_extra) for evaluation runs.model_version) are protected from being overridden by user-supplied metadata.testing_frameworkwhen no run metadata is provided.run_metadata_extrais omitted.