[OPIK-8059] [BE][FE] fix: score playground dataset runs by the selected rules - #7989
[OPIK-8059] [BE][FE] fix: score playground dataset runs by the selected rules#7989jverre wants to merge 7 commits into
Conversation
…ed rules An experiment trace is now scored by the rules selected in the playground, whatever their trigger scope, enabled flag, filters or sampling rate, and by every enabled rule scoped to experiments. Playground traces without a dataset are scored as production traffic. The playground no longer selects every rule by default, and the output cell renders every score that arrives instead of only the selected ones. Implements OPIK-8059: Online evaluation rule with "Production traces" trigger scope never scores a Playground dataset run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
java
⏱️ pre-commit per-hook timing
⏭️ 40 skipped (no matching files changed)
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
The refetch predicate stopped as soon as the selected rules had reported, so scores from enabled rules targeting experiments arrived after the last fetch and never reached the cell. Polling now waits for those rules too, while the pending tags still come from the selected rules alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
| const awaitedScoreNamesRef = useRef<Set<string>>(new Set()); | ||
| awaitedScoreNamesRef.current = new Set(scoreNamesOf(scoringRules)); |
There was a problem hiding this comment.
scoreNamesOf(scoringRules) reparses full code.metric for every rule on each render, and PlaygroundOutputTable mounts a PlaygroundOutputScoresContainer per cell, so trace polling repeatedly scans unbounded evaluator source — should we memoize the result with useMemo([scoringRules]) before assigning to awaitedScoreNamesRef.current and bound the source size or return score-name metadata from the server?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputScores/PlaygroundOutputScoresContainer.tsx`
around lines 81-82, the polling setup recomputes `scoreNamesOf(scoringRules)` on every
render, fully re-parsing each rule's `code.metric` and allocating a new `Set`. First,
wrap this derived awaited-score-name Set in `useMemo`, keyed by `scoringRules`, and
assign the memoized Set to `awaitedScoreNamesRef.current` so re-renders avoid redundant
scanning/parsing/allocation. Second, since `PlaygroundOutputTable` renders a
`PlaygroundOutputScoresContainer` per cell and the evaluator DTO only enforces
`@NotNull` (no size limit), large Python rules get repeatedly parsed by `scoreNamesOf`
across many cells during polling — enforce a strict maximum source length at the
evaluator create/update DTO or persistence boundary, and/or have the flow consume
bounded, server-provided score-name metadata instead of parsing unbounded `code.metric`
text in the render/polling path.
There was a problem hiding this comment.
Commit 1a086df addressed this comment by memoizing the derived awaited score-name set with useMemo([scoringRules]) and storing that memoized set in the ref. The requested source-size bound or server-provided metadata was not added.
There was a problem hiding this comment.
Correct, and fixed in 1a086df. The extraction now sits in a useMemo keyed on the rule set, so the Python source is parsed only when the rules change rather than on every render of every cell.
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
…e names The rules list carries thread and span rules too. Their names could never arrive in Trace.feedback_scores, so awaiting them kept the cell polling to the 300s ceiling. Restrict the awaited set to the trace-level rule types, which is what the trace sampler publishes. The name extraction also ran on every render, and it parses Python rule source character by character, once per output cell. Memoize it on the rule set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Already covered by a test in this PR. Both halves of this are already pinned by specs in QA draft #8000: Also already tested. Also worth a test. A ticked rule now bypasses areas: Advisory, from the QA test radar. Nothing here blocks this PR, and anything it proposes is a draft for review. Re-checked after a push on 26 Aug 14:36 UTC — nothing the verdict depends on changed. |
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
miguelgrc
left a comment
miguelgrc
left a comment
There was a problem hiding this comment.
Opik reviewer (mined from your team's review history)
6 findings — 1 high · 4 medium · 1 low. Suppressed by team conventions: see suppressed.md.
React 👍/👎 on each comment — your feedback helps tune what it flags.
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
A playground run with no dataset is a scratchpad, not production traffic, so production-scoped rules should not judge it. Only experiment traces and SDK traces are scored now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Backend Tests - Integration Group 16 30 files + 3 30 suites +3 3m 18s ⏱️ +19s For more details on these errors, see this check. Results for commit 336f1df. ± Comparison against base commit 14f554b. This pull request removes 36 and adds 45 tests. Note that renamed tests count towards both.This pull request removes 1 skipped test and adds 3 skipped tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
| } else if (Source.isLoggingSource(trace.source())) { | ||
| scorableTraces.add(trace); | ||
| } |
There was a problem hiding this comment.
Source.isLoggingSource accepts only SDK and null, so standalone Source.PLAYGROUND traces are filtered before reaching ruleEvaluatorService.findAll or message publication — should we confirm whether they should score? If filtering is intentional, should we switch OnlineScoringEngineTest.testFilteringEvaluatorsByTraceMetadata to Source.EXPERIMENT and add a real TraceService/EventBus assertion that playground traces with selected_rule_ids are not scored; otherwise include Source.PLAYGROUND while preserving the production trigger checks?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code and clarify intended
behavior for `Source.PLAYGROUND` traces in
`apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/events/OnlineScoringSampler.java`
around lines 233-235. Option A (if playground traces should be scorable): update the
`sampleAndScore` trace eligibility logic so standalone `Source.PLAYGROUND` traces are
included when no dataset is selected, extending the fallback condition or helper
(currently based on `Source.isLoggingSource`) to accept PLAYGROUND, while preserving the
existing production trigger checks and selected-rule handling. Option B (if the
PLAYGROUND filtering is intentional): keep the `Source.isLoggingSource` gate as-is, but
update `OnlineScoringEngineTest.testFilteringEvaluatorsByTraceMetadata` to use
`Source.EXPERIMENT` instead of `Source.PLAYGROUND` so it continues validating scoring
with selected rule IDs, and add a separate test asserting that a playground trace with
selected IDs produces no scoring — preferably exercising the real
`TraceService`/`EventBus` flow rather than manually constructing the event and mocking
the score service.
There was a problem hiding this comment.
Commit 336f1df addressed this comment by explicitly limiting selected-rule scoring to Source.EXPERIMENT and excluding standalone playground traces, while preserving logging-source scoring. The requested dedicated regression test is not shown.
There was a problem hiding this comment.
Filtering is intentional and was decided explicitly: a playground run without a dataset is a scratchpad, not production traffic, so it is not scored at all. The ticket description records the contract. OnlineScoringEngineTest.testFilteringEvaluatorsByTraceMetadata is switched to Source.EXPERIMENT in 2f82d81, as you suggest. The "playground traces are not scored" assertion already exists in OnlineScoringSamplerTest.skipsTracesFromNonScorableSources, which is parameterized over every source except SDK and EXPERIMENT — so PLAYGROUND, OPTIMIZATION and EVALUATOR are all covered.
There was a problem hiding this comment.
Thanks for clarifying the intended scratchpad behavior and existing coverage. I’ll save this context to memory once the PR is merged.
With nothing selected and no experiment-scoped rules, every output cell polled its trace every 5s for the full 5 minutes with nothing to wait for. Gate the query on there being a rule that will score the trace, which is distinct from being able to predict its score names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Backend Tests - Integration Group 13 33 files - 22 33 suites - 22 5m 37s ⏱️ +17s For more details on these errors, see this check. Results for commit 336f1df. ± Comparison against base commit 14f554b. This pull request removes 121 and adds 85 tests. Note that renamed tests count towards both. |
| const { data: rulesData, isSuccess: rulesLoaded } = useRulesList( | ||
| { | ||
| workspaceName, | ||
| projectId: activeProjectId ?? undefined, |
There was a problem hiding this comment.
Rules beyond first 100 never refresh scores
The component fetches only page 1 with size: 100 even though useRulesList returns total, so an enabled experiment/both rule beyond the first 100 is missing from scoringRules, leaving hasScoringRules false and disabling useTraceById; should we paginate all rules or use the existing all-rules loading pattern so late feedback scores render?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputScores/PlaygroundOutputScoresContainer.tsx`
around lines 47-53, update the `PlaygroundOutputScoresContainer` rule-loading logic so
it does not inspect only page 1 with `size: 100`. Fetch all rule pages using the
pagination metadata, or adopt the existing sufficiently large/all-rules loading pattern,
before computing `scoringRules` and `hasScoringRules`, ensuring experiment/both rules
beyond the first page keep `useTraceById` enabled.
There was a problem hiding this comment.
Addressed in 2f82d81, though not by paginating. The gate now also checks total > rules.length: when the list is truncated we cannot know whether a scoring rule sits beyond page 1, so polling stays on. That keeps late scores rendering without a second request per cell. Fetching every page is the fuller fix and belongs with the select-all pagination gap tracked separately.
There was a problem hiding this comment.
Commit 2f82d81 addressed this comment by keeping polling enabled when the rules response is truncated, so scoring rules beyond the first page no longer disable useTraceById.
| const scoringRules = useMemo( | ||
| () => | ||
| rules.filter( | ||
| (rule) => | ||
| TRACE_RULE_TYPES.includes(rule.type) && | ||
| (selectedRuleIdsSet.has(rule.id) || | ||
| (rule.enabled !== false && | ||
| (rule.trigger_scope === EVAL_TRIGGER_SCOPE.experiment || | ||
| rule.trigger_scope === EVAL_TRIGGER_SCOPE.both))), |
There was a problem hiding this comment.
Disabled Python rules cause endless polling
scoringRules includes user_defined_metric_python rules even when ServiceTogglesConfig.pythonEvaluatorEnabled is false, so the backend produces no score for them while the frontend polls indefinitely — should we apply the same toggle when building scoringRules, or carry the backend capability into this decision?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputScores/PlaygroundOutputScoresContainer.tsx`
around lines 75-83, update the `scoringRules` derivation so `user_defined_metric_python`
rules are included only when the backend Python evaluator capability is enabled. Obtain
this capability from the existing frontend configuration or API state, and ensure
Python-only rules do not enable polling when the backend cannot publish their scores; if
appropriate, expose the disabled capability to the user instead.
There was a problem hiding this comment.
Not fixable from here: pythonEvaluatorEnabled is a server-side toggle the frontend cannot observe, and there is no endpoint exposing it. It is also unchanged from main, where a selected Python rule with the toggle off polled to the same ceiling. Leaving as is; the cost is polling duration, not a wrong result.
There was a problem hiding this comment.
Thanks for the clarification—I'll remember this constraint after the PR is merged.
| // With no rule to score this trace there is nothing to poll for. Until the rules arrive we | ||
| // cannot tell, so the query stays enabled while the list is still loading. | ||
| const rulesPending = !!activeProjectId && !rulesLoaded; | ||
| const hasScoringRules = rulesPending || scoringRules.length > 0; |
There was a problem hiding this comment.
Rules errors cause prolonged trace polling
rulesPending relies on !isSuccess, so a terminal useRulesList error is still treated as pending, keeping hasScoringRules true and useTraceById polling every 5 seconds for up to 300,000 ms while the failure is silently ignored — should we check isError to stop polling and surface the error instead?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputScores/PlaygroundOutputScoresContainer.tsx`
around lines 88-91, fix the `rulesPending`/`hasScoringRules` logic so a terminal
`useRulesList` error is not treated as an unknown pending rules set that enables
`useTraceById` polling. Read the query's error state, disable trace fetching when rule
discovery fails, and surface the rules-loading error through the component's UI or error
state instead of silently continuing requests.
There was a problem hiding this comment.
Correct, and fixed in 2f82d81. rulesPending now excludes the error state, so a failed rules lookup stops polling rather than counting as still-loading. Surfacing the error in the cell is a separate concern — there is no UI slot for it today.
There was a problem hiding this comment.
Thanks, that addresses the polling issue. I’ll save this context to memory once the PR is merged.
There was a problem hiding this comment.
Commit 2f82d81 addressed this comment by excluding failed rule queries from rulesPending, preventing trace polling from continuing to the timeout. It does not appear to surface the rules error in the UI.
| const { data: trace } = useTraceById( | ||
| { traceId: traceId! }, | ||
| { | ||
| enabled: !!traceId && hasRulesSelected, | ||
| enabled: !!traceId && hasScoringRules, | ||
| refetchInterval: (query) => { |
There was a problem hiding this comment.
Large runs create unbounded trace polling fan-out
The broadened enabled gate runs useTraceById for every mounted score cell, so a 1,000-row page with N prompt columns creates 1,000 × N polling loops and a request load without a fixed budget — should we virtualize the score cells or impose a concurrency/request budget before enabling per-cell polling?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputScores/PlaygroundOutputScoresContainer.tsx`
around lines 106-110, the `PlaygroundOutputScoresContainer` enables an independently
polling `useTraceById` query for every mounted score cell, causing request volume to
grow with the number of rows and prompt columns. Refactor the output rendering/query
coordination to virtualize score cells or enforce a fixed concurrency and request budget
before enabling per-cell polling, so a 1,000-row page and many prompts cannot create
unbounded polling traffic. Preserve the existing score completion and timeout behavior
within that bound.
There was a problem hiding this comment.
One container per cell is the existing shape, not something this PR introduces, and the gate narrows it rather than widening it. On main every cell polled unconditionally for the full 300s, including when the project had no rules at all. Here a cell polls only when a rule can actually score the trace. Virtualizing the score cells is a table concern and out of scope for this change.
There was a problem hiding this comment.
Thanks for the context—this gate narrows the existing polling behavior, and score-cell virtualization is outside this change’s scope. I’ll save this to memory once the PR is merged.
Backend Tests - Integration Group 765 tests - 1 596 55 ✅ - 1 606 10m 24s ⏱️ +12s For more details on these failures and errors, see this check. Results for commit 336f1df. ± Comparison against base commit 14f554b. This pull request removes 1623 and adds 27 tests. Note that renamed tests count towards both. |
OnlineScoringEngineTest still encoded the old playground contract: it built a playground trace naming two of three rules and asserted six scores. Playground traces are no longer scored, so it now uses an experiment trace, where the two picked rules score and the third, scoped to production, stays out. The polling gate treated a failed rules lookup as still loading, which kept the cell polling to the timeout, and it could not tell that the rules list is capped at one page. Stop on a failed lookup, keep polling when the list is truncated. Also corrects a comment that described null selection as a legacy value when the store still writes it, and adds sampler coverage for a picked Python rule with the evaluator toggle on and off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| // Metric selection is a plain list of rule ids. Both an empty list and null mean nothing is | ||
| // selected -- the store still writes null whenever a dataset has no stored selection -- and the | ||
| // run is then scored only by the rules that target experiments. |
There was a problem hiding this comment.
Persisted selections silently lose all rules
Persisted PLAYGROUND_STATE entries with scoresByDatasetId[datasetId] = null now mean “none selected” instead of the legacy “all metrics selected,” so RunExperimentControl reads them with ?? null and createLogPlaygroundProcessor omits selected_rule_ids, letting upgraded users run existing datasets without the rules previously selected by default. Should we add an explicit migration/versioned fallback for legacy null, or document and intentionally gate this clean break?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/metricSelection.ts` around lines 1-3,
preserve the legacy meaning of persisted `scoresByDatasetId[datasetId] = null` as “all
metrics selected” instead of redefining it as “none selected” for existing
`PLAYGROUND_STATE` entries. Add an explicit persisted-state migration/version marker or
a legacy fallback before `RunExperimentControl` and `createLogPlaygroundProcessor`
consume the selection, while retaining `[]` for an intentional empty selection. Update
the relevant tests to cover upgraded datasets with legacy `null` values.
| const { | ||
| data: rulesData, | ||
| isSuccess: rulesLoaded, | ||
| isError: rulesFailed, | ||
| } = useRulesList( |
There was a problem hiding this comment.
Score polling state machine lacks coverage
useRulesList/useTraceById lack a focused component/query-seam test, so their loading/error, traceId/activeProjectId null, pagination/name-resolution, and rule-scope branches that control polling and rendered metric sets remain unverified. Should we add a mocked-hook container/query test with explicit assertions for each scenario, as required by .agents/skills/opik-frontend/testing.md and AGENTS.md?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputScores/PlaygroundOutputScoresContainer.tsx`
around lines 47-51, add a focused test for the `useRulesList`/`useTraceById`
state-machine seam used by `PlaygroundOutputScoresContainer`. Mock both hooks and assert
`enabled`, polling behavior, and rendered metric names for loading, terminal-error, and
null `traceId`/`activeProjectId` cases, including `total > content.length` and awaited
names already present. Also cover selected, implicit experiment-scoped, thread/span, and
Python rule combinations, following the frontend testing guidance and repository test
organization rules.
| const rulesPending = !!activeProjectId && !rulesLoaded && !rulesFailed; | ||
| const rulesTruncated = (rulesData?.total ?? 0) > rules.length; | ||
| const hasScoringRules = | ||
| rulesPending || rulesTruncated || scoringRules.length > 0; |
There was a problem hiding this comment.
Polling stops after a failed useRulesList lookup or when page-1 awaitedScoreNames is present despite rulesTruncated, while the backend's unpaginated findAll(projectId, workspaceId) can still evaluate later-page rules, so their scores are never fetched or rendered — should we keep polling through MAX_REFETCH_TIME for unknown rule state and while rulesTruncated is true, or load all pages first?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages/PlaygroundPage/PlaygroundOutputs/PlaygroundOutputScores/PlaygroundOutputScoresContainer.tsx`
around lines 95-98, fix the `rulesPending`/`hasScoringRules` gating logic used to
control `refetchInterval` for the trace query so it doesn't stop polling prematurely in
two scenarios: (1) a failed `useRulesList` request should be treated as an unknown rule
state rather than a confirmed absence of scoring rules, keeping the trace query enabled
and polling through the existing `MAX_REFETCH_TIME` timeout; (2) polling should not stop
once page-1 `awaitedScoreNames` is present if `rulesTruncated` is true (i.e.,
`rulesData.total > rules.length`), since the backend's unpaginated `findAll(projectId,
workspaceId)` may still evaluate rules beyond page 1. Only stop polling once a
successful, complete rule lookup (all pages loaded) confirms there are no scoring rules,
or explicitly keep polling while `rulesTruncated` is true.
| var trace = createTrace(traceId, projectId, Source.EXPERIMENT).toBuilder() | ||
| .metadata(metadata) | ||
| .build(); |
There was a problem hiding this comment.
Cross-layer scoring contract remains untested
The new engine test constructs Trace directly with Source.EXPERIMENT and selected_rule_ids, bypassing buildLogProcessor/snakeCaseObj and POST /v1/private/traces/batch, so serialization or selection regressions can leave tests green while changing which rules are scored — should we add a producer-to-ingestion contract test covering experiment runs with [] and selected IDs, scratchpad playground runs with null/[], and applicable malformed or legacy-null inputs, as AGENTS.md requires?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/events/OnlineScoringEngineTest.java`
around lines 453-455, update `testFilteringEvaluatorsByTraceMetadata` so it does not
rely solely on a directly constructed `Trace` with `Source.EXPERIMENT` and
`selected_rule_ids`. Add a focused producer-to-ingestion or public `POST
/v1/private/traces/batch` integration test that exercises frontend serialization and
deserialization for experiment traces with empty and selected rule IDs,
scratchpad/playground traces with null and empty selections, and applicable malformed or
legacy-null inputs. Assert the resulting scoring behavior and source/selection fields so
regressions across the boundary cannot leave the engine tests passing.
| void scoresPickedPythonRuleOnExperimentTracesWhenToggleIsEnabled() { | ||
| when(serviceTogglesConfig.isPythonEvaluatorEnabled()).thenReturn(true); | ||
| var evaluator = createPythonEvaluator(0.0f, EvalTriggerScope.PRODUCTION); | ||
| var trace = createTrace(Source.EXPERIMENT).toBuilder() | ||
| .metadata(metadataWithRuleIds(evaluator.getId())) | ||
| .build(); | ||
| whenFindAllPythonEvaluators(evaluator); | ||
|
|
||
| onlineScoringSampler.onTracesCreated(new TracesCreated(List.of(trace), workspaceId, userName)); | ||
|
|
||
| verify(onlineScorePublisher).enqueueMessage(List.of(toPythonMessage(evaluator, trace)), | ||
| AutomationRuleEvaluatorType.USER_DEFINED_METRIC_PYTHON); |
There was a problem hiding this comment.
Duplicated cases drift over time
The enabled and disabled toggle cases duplicate the evaluator, trace, stubbing, invocation, and message setup, so equivalent permutations can drift in coverage when fixtures change — should we consolidate them into a @ParameterizedTest/@MethodSource supplying the toggle value and VerificationMode (times(1) versus never()), while sharing the exact message verification?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/events/OnlineScoringSamplerTest.java`
around lines 280-297, refactor
`scoresPickedPythonRuleOnExperimentTracesWhenToggleIsEnabled` and
`skipsPickedPythonRuleOnExperimentTracesWhenToggleIsDisabled` into one
`@ParameterizedTest` backed by a `@MethodSource`. Supply the Python evaluator toggle
value and the appropriate Mockito `VerificationMode` (`times(1)` or `never()`), keeping
the shared evaluator, trace, stubbing, invocation, and message-verification setup
centralized so both cases retain equivalent coverage.
|
🌙 Nightly cleanup: The test environment for this PR ( |
3 participants
Details
Before
source: experiment.selected_rule_idson experiment traces and applied the trigger scope instead.After
source = experiment:selected_rule_idsscores the trace, whatever its trigger scope, enabled flag, filters or sampling rate.experimentorbothalso scores it, ignoring filters and sampling.source = sdkor null: enabled rules scopedproductionorbothscore the trace, with filters and sampling applied.selected_rule_idsis not read.playgroundincluded: not scored. A playground run without a dataset is a scratchpad, not production traffic.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
mvn test -Dtest=OnlineScoringSamplerTest— 50 tests, 0 failures. Covers a selected rule that is disabled, filtered and sampled out; an unselected experiment-scoped rule scoring alongside the selected one; a rule qualifying on both paths enqueuing once;selected_rule_idsignored on production traffic; and non-scorable sources skipped, playground among them.mvn spotless:check— clean.npm run lintandnpm run typecheck— clean.npx vitest run src/v2/pages/PlaygroundPage— 17 tests, 0 failures.Documentation
N/A