Skip to content

feat(online-scoring): enable agentic-tools by default for all deployments - #7950

Open
aadereiko wants to merge 1 commit into
mainfrom
sasha/agentic-tools-default-on
Open

feat(online-scoring): enable agentic-tools by default for all deployments#7950
aadereiko wants to merge 1 commit into
mainfrom
sasha/agentic-tools-default-on

Conversation

@aadereiko

Copy link
Copy Markdown
Collaborator

Why

The agentic-tools path on LLM-as-judge online scoring only ever reached Comet-hosted environments, which pin TOGGLE_AGENTIC_TOOLS_ENABLED=true in their chart values. Self-hosted installs inherit the image's config.yml default — and that default was still "false", so neither the docker-compose nor the helm install path ever turned the feature on. Enabling it for self-hosted was requested.

What

Flip the serviceToggles.agenticToolsEnabled default in apps/opik-backend/config.yml from "false" to "true".

That single line covers every self-hosted install, because neither deployment path names this variable:

Path Env source Pins TOGGLE_AGENTIC_TOOLS_ENABLED?
docker-compose deployment/docker-compose/docker-compose.yaml No — only TOGGLE_OLLIE_ENABLED, TOGGLE_OPIK_AI_ENABLED, TOGGLE_GUARDRAILS_ENABLED, TOGGLE_WELCOME_WIZARD_ENABLED
helm component.backend.envconfigmap-backend.yaml No

configmap-backend.yaml only emits keys present in component.backend.env, so an absent key means no env var in the pod and the image default applies. Operators keep full control — adding component.backend.env.TOGGLE_AGENTIC_TOOLS_ENABLED: "false" still wins — while the image retains the ability to move the default, which is exactly the property a chart-pinned value would destroy. Hence no chart change here.

Test config

config-test.yml mirrors the flip so app-booting tests exercise the path real deployments now run. Previously CI validated the inline path while we were about to ship the agentic one as the default.

Deliberately unchanged

  • Frontend DEFAULT_STATE stays false (feature-toggles-provider.tsx). It only applies when the backend omits the key — a newer FE against an older BE with no agentic support. Flipping it would enable UI affordances the backend can't honour.
  • Helm values.yaml — see the table above. Also worth noting no TOGGLE_* key appears in the generated chart README for any component, so listing just this one would be inconsistent; documenting all toggles is a separate change.

Testing

Run locally against the flipped config-test.yml:

Class Result
TraceThreadOnlineScoringAgenticToolsE2ETest ✅ 1/1
TraceThreadOnlineScoringSamplerListenerIntegrationTest ✅ 6/6
OnlineScoringSpanSamplerIntegrationTest ✅ 2/2
AutomationRuleEvaluatorsResourceTest 135/136 — one flaky ConditionTimeout, see below
ManualEvaluationResourceTest ✅ 20/20

The AutomationRuleEvaluatorsResourceTest failure is not attributable to this change, on three grounds:

  1. It landed on a different test each rungetLogsUserDefinedMetricPythonScorer, then getLogsTraceSkippedDueToDisabledRule — each passing in the other run.
  2. Single-method A/B runs passed with the toggle at both false and true.
  3. The second failure's test creates both rules with enabled(false), so no scorer is ever invoked and a flag governing scorer context-building cannot reach it.

Every failure had the shape Expected size: N but was: 0 … within 10 seconds — zero logs, i.e. the Awaitility window losing a race under class-level load, not a behavioural divergence (which would emit logs with diverging content).

🤖 Generated with Claude Code

…ents

The agentic-tools path on LLM-as-judge online scoring was only reaching
Comet-hosted environments, which pin TOGGLE_AGENTIC_TOOLS_ENABLED=true in
their chart values. Self-hosted installs inherit the image's config.yml
default, and that default was still "false", so neither the docker-compose
nor the helm install path ever turned the feature on.

Flip the config.yml default to "true" so every deployment gets it. Neither
deployment/docker-compose/docker-compose.yaml nor the helm chart's
component.backend.env names this variable, so both inherit the new default
with no chart change; operators can still force the inline path by setting
TOGGLE_AGENTIC_TOOLS_ENABLED=false.

config-test.yml mirrors the flip so app-booting tests exercise the path real
deployments run. The unit tests are unaffected (they mock ServiceTogglesConfig
and stub isAgenticToolsEnabled per case), and TraceThreadOnlineScoringAgenticTools
E2ETest already forced the toggle on via customConfigs.

The frontend DEFAULT_STATE stays false on purpose: it only applies when the
backend omits the key, i.e. a newer FE against a BE with no agentic support.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@aadereiko
aadereiko requested a review from a team as a code owner August 21, 2026 14:28
@github-actions github-actions Bot added Backend tests Including test files, or tests related like configuration. labels Aug 21, 2026
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

📋 PR Linter Failed

Invalid Title Format. Your PR title must include a ticket/issue number and may optionally include component tags ([FE], [BE], etc.).

  • Internal contributors: Open a JIRA ticket and link to it: [OPIK-xxxx] or [CUST-xxxx] or [DND-xxxx] or [DEV-xxxx] [COMPONENT] Your change
  • External contributors: Open a Github Issue and link to it via its number: [issue-xxxx] [COMPONENT] Your change
  • No ticket: Use [NA] [COMPONENT] Your change (Issues section not required)

Example: [issue-3108] [BE] [FE] Fix authentication bug or [OPIK-1234] Fix bug or [NA] Update README


Missing Section. The description is missing the ## Details section.


Missing Section. The description is missing the ## Change checklist section.


Missing Section. The description is missing the ## Issues section.


Missing Section. The description is missing the ## Documentation section.

@github-actions

Copy link
Copy Markdown
Contributor

⏱️ pre-commit per-hook timing

No linted files changed — nothing to run.

⏭️ 43 skipped (no matching files changed)
Hook Description Result
🐍 trim trailing whitespace — python sdk Strip trailing whitespace ⏭️
🐍 fix end of files — python sdk Ensure files end in a newline ⏭️
🐍 ruff — python sdk Lint + autofix Python (ruff) ⏭️
🐍 ruff-format — python sdk Format Python code (ruff) ⏭️
🐍 mypy — python sdk Static type check ⏭️
🤖 trim trailing whitespace — optimizer Strip trailing whitespace ⏭️
🤖 fix end of files — optimizer Ensure files end in a newline ⏭️
🤖 check yaml — optimizer Validate YAML syntax ⏭️
🤖 check json — optimizer Validate JSON syntax ⏭️
🤖 check toml — optimizer Validate TOML syntax ⏭️
🤖 check for added large files — optimizer Block large files (>1MB) ⏭️
🔐 detect private key — optimizer Block committed private keys ⏭️
🤖 check for merge conflicts — optimizer Block merge-conflict markers ⏭️
🤖 check for case conflicts — optimizer Block case-only name clashes ⏭️
🤖 pyupgrade — optimizer Modernize Python syntax ⏭️
🤖 ruff — optimizer Lint + autofix Python (ruff) ⏭️
🤖 ruff-format — optimizer Format Python code (ruff) ⏭️
🤖 mypy — optimizer Static type check ⏭️
📓 nbstripout — optimizer notebooks Strip notebook output ⏭️
📝 markdownlint — optimizer Lint Markdown ⏭️
🔤 codespell — optimizer Fix common misspellings ⏭️
📊 radon cc — optimizer Cyclomatic-complexity gate ⏭️
📊 radon raw — optimizer Raw size metrics gate ⏭️
📊 xenon — optimizer Fail on complexity thresholds ⏭️
📊 lizard — optimizer Cyclomatic-complexity gate ⏭️
🧹 vulture — optimizer Find dead code ⏭️
🛡️ trim trailing whitespace — guardrails Strip trailing whitespace ⏭️
🛡️ fix end of files — guardrails Ensure files end in a newline ⏭️
🛡️ ruff — guardrails Lint + autofix Python (ruff) ⏭️
🛡️ ruff-format — guardrails Format Python code (ruff) ⏭️
🛡️ mypy — guardrails Static type check ⏭️
⚓ helm-docs Regenerate Helm chart README ⏭️
block non-public FE plugins Block non-public FE plugins ⏭️
☕ spotless — java backend Format Java code ⏭️
🧪 pre-commit wrapper smoke tests Self-test the wrapper scripts ⏭️
🧪 rebaseline script tests Self-test the changelog re-baseline script ⏭️
🌐 eslint — frontend Lint + autofix JS/TS ⏭️
🌐 typecheck — frontend Whole-project tsc type check ⏭️
📘 eslint — typescript sdk Lint + autofix JS/TS ⏭️
📘 typecheck — typescript sdk Whole-project tsc type check ⏭️
⚙️ actionlint — github workflows Lint GitHub Actions workflows ⏭️
🐳 hadolint — dockerfiles Lint Dockerfiles ⏭️
🌈 zizmor — github workflows security Security-scan GitHub Actions workflows ⏭️

@CometActions

Copy link
Copy Markdown
Collaborator

This change looks worth a test.

Flipping agenticToolsEnabled to true changes what a default OSS install does: GET /v1/private/toggles/ now returns agentic_tools_enabled: true, and the LLM-judge rule dialog reads that toggle — {{spans}} / {{trace}} in a trace-scope prompt now auto-fill as reserved sentinels instead of staying empty, user-mappable rows. online-evaluation-smoke.spec.ts is tagged @cap:online-evaluation.create-llm-judge-rule, but it builds the rule from the canned Moderation template, which has no reserved variables, and its 5 seeded traces are nowhere near the 50k-token routing threshold — it passes identically with the toggle on or off, and nothing else under tests_end_to_end/ mentions spans, trace or the toggles endpoint. Worth pinning the dialog behaviour and the toggle default. The agentic-tools loop itself needs a provider key and a 50k-token trace, so that part stays manual either way.

Would target online-evaluation.create-llm-judge-rule.

What it would check
  1. On a stock install with no env overrides, GET /v1/private/toggles/ and confirm agentic_tools_enabled is true (it was false before this PR)
  2. Open a project > Online evaluation > Create rule > LLM-as-judge, trace scope, and replace the prompt with one containing {{spans}} and {{trace}}; confirm both variables appear pre-mapped to their sentinels instead of as empty editable rows
  3. In the same dialog, add a non-reserved variable like {{input}} and confirm it still renders as a normal empty row the user maps by hand — the auto-fill must be limited to the reserved names
  4. Switch the dialog to span scope and confirm {{span}} auto-fills while {{spans}}/{{trace}} do not; switch to thread scope and confirm no variables section renders at all
  5. Save a rule carrying {{spans}}, reopen it for edit, and confirm the persisted variables.spans mapping survives the round-trip rather than being re-derived or cleared

Deploying a test environment for this PR and exploring it — results will follow in a comment.

also touches Backend (Java API / internal)
Run

Advisory, from the QA test radar. Nothing here blocks this PR, and anything it proposes is a draft for review.

@CometActions CometActions added the test-environment Deploy Opik adhoc environment label Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔄 Test environment deployment process has started

Phase 1: Deploying base version 2.2.37-6395 (from main branch) if environment doesn't exist
Phase 2: Building new images from PR branch sasha/agentic-tools-default-on
Phase 3: Will deploy newly built version after build completes

You can monitor the progress here.

Comment on lines +1471 to +1476
# Default: true
# Description: Master switch for the agentic-tools path on LLM-as-judge online scoring. When false, the
# inline path is used regardless of context size. Threshold lives under onlineScoring.agenticToolsThresholdTokens.
agenticToolsEnabled: ${TOGGLE_AGENTIC_TOOLS_ENABLED:-"false"}
# On by default for every deployment, self-hosted included; set TOGGLE_AGENTIC_TOOLS_ENABLED=false to force
# the inline path (e.g. to keep online scoring on a single LLM round-trip).
agenticToolsEnabled: ${TOGGLE_AGENTIC_TOOLS_ENABLED:-"true"}

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.

Pathological tools block scoring workers

Defaulting agenticToolsEnabled to true runs model-controlled JqTool expressions and SearchTool regexes synchronously on the scheduler with no complexity or size limits, so a pathological expression can occupy a scheduler thread indefinitely before the round/output caps are even checked — should agentic tools stay opt-in by default, and should we add expression/pattern complexity limits plus an enforceable timeout or isolated worker budget?

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
apps/opik-backend/config.yml around lines 1471-1476, update the `agenticToolsEnabled`
online-scoring feature flag so agentic tools remain opt-in by default rather than
enabling potentially unbounded `JqTool` and `SearchTool` execution for every deployment.
Also harden the corresponding tool execution paths with expression/pattern size and
complexity limits plus an enforceable timeout or isolated worker resource budget; do not
rely on the existing round/output caps, which are checked only after execution
completes.

@CometActions

Copy link
Copy Markdown
Collaborator

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.

@CometActions

Copy link
Copy Markdown
Collaborator

🌙 Nightly cleanup: The test environment for this PR (pr-7950) has been cleaned up to free cluster resources. PVCs are preserved — re-deploy to restore the environment.

@CometActions CometActions removed the test-environment Deploy Opik adhoc environment label Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend 🔵 size/XS tests Including test files, or tests related like configuration.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants