发布

RSS 订阅源

2.2.1 e27644deb3

比较
[OPIK-7172] [BE][FE][SDK] feat: re-expose custom-code metric in Optimization Studio (#7460)

frostbyte_neo 发布于 2026-07-21 10:04:22 +00:00
- [OPIK-7172] [BE][FE][SDK] feat: re-expose custom-code metric in Optimization Studio
Re-expose the custom-code metric (METRIC_TYPE.CODE) in the v2 Optimization
Studio new-run flow as a first-class, validated option, with proper error
surfacing.

Backend (python + java):
- Fix build-time false-rejection: validate custom code via compile()/exec +
  instantiate to read the metric name WITHOUT calling score() (was running
  score() with an empty payload, wrongly rejecting metrics that require dataset
  fields). Error taxonomy mirrors the online-eval sandbox.
- Rename-capable arguments map (param -> dataset column): isolated_metric builds
  score() kwargs from the map; for strict signatures (no **kwargs) only mapped
  params + output are passed (no full-splat), preserving **kwargs back-compat.
- Persist an error_info field on the Optimization record (ClickHouse migration
  000102 + DAO carry-through on status-only re-inserts + model/update DTOs);
  mark_error(message) threads the reason; blank messages don't clobber.
Frontend (v2):
- Re-add the "Custom code" metric option.
- Client-side Python syntax validation via the CodeMirror/Lezer grammar.
- score() signature -> arg/column mapping UI with dataset-column validation;
  kwargs.get("x", default) no longer treated as a required column.
- Error callout + wire the run-page Logs panel on ERROR status.
- Recommend kwargs.get(...) in helper copy; consolidate the code template.
SDK: thread error_info through optimizations update + optimizer finalize.

Tests: BE unit (syntax error, missing BaseMetric, runtime->0.0, rename with
extra columns) + e2e negatives; FE unit for the validation blocks.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: CI — ruff-format optimizer + updateById test errorInfo
- ruff-format base_optimizer.py + test_base_optimizer_finalize.py (lint check).
- OptimizationsResourceTest.updateById: errorInfo is now an updatable field on
  OptimizationUpdate, so the expected optimization must reflect the applied
  value (update value if non-null, else keep existing) — mirrors the name/status
  assertion and the DAO's <if(error_info)> update semantics.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: surface error status when pinned opik SDK lacks error_info
Root cause of the stuck-"running" e2e failure: the python-backend pins a
released opik (1.10.56) whose typed update_optimizations_by_id has no
error_info kwarg. On a build-time failure, mark_error(error_info=...) raised
TypeError, which optimization_lifecycle swallows — so no "error" row was
written and the run hung at "running".

Fix: status update tries the typed call, and on TypeError falls back to the
SDK's pre-configured httpx client (raw PUT) so error_info still reaches the
backend (snake_case + ignoreUnknown). Forward-compatible: once the SDK ships
error_info, the typed call handles it and the fallback is never used.

Reproduced end-to-end against a local branch backend (migration 000102 +
error_info column): before, the run stayed "running" with no error row; after,
it reaches "error" with the reason persisted, and the e2e test passes.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: rename optimizations error_info migration 000102 -> 000103
main merged 000102_add_harness_to_cipx_trace_identity after this branch was
cut, so 000102_add_error_info_to_optimizations collided (check-migration-prefix-
conflicts). Bump to the next free prefix 000103 + matching changeset id.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: address review — static build validation, strict-signature safety
Baz review findings:
- [high] In-process exec of untrusted code: build-time validate_user_code now
  validates purely via ast.parse (no exec / no init), so no untrusted
  user code runs at build; score() at scoring time still honors
  PYTHON_CODE_EXECUTOR_STRATEGY. AST also yields the metric name, **kwargs
  presence, and score() param names.
- [high] Strict code metrics masked as 0.0: isolated_metric now branches on the
  **kwargs signature — a strict score() gets ONLY its declared params (resolved
  via the arguments map or a same-name column), never the full dataset splat —
  whether or not an arguments map is present. Prevents the extra-column
  TypeError -> swallowed 0.0.
- [low] Invalid mapping types: non-string 'arguments' entries are dropped before
  the dataset-key lookup.
- [high/defensive] mapToDto defaults error_info to "" for robustness on reads.
Tests: 18 code-metric unit tests pass (added strict-no-map + non-string-args
guards); syntax-error e2e still green (reproduced against a local branch backend).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] refactor: optimization error_info as structured ErrorInfo
Address review (alexkuzmik): the run failure reason now uses the existing
structured ErrorInfo type {exception_type, message, traceback} — the same type
spans/traces use — instead of a plain string, so it's consistent and richer to
render.
- Java: Optimization/OptimizationUpdate error_info String -> ErrorInfo; DAO
  serializes to / reads from the ClickHouse String column as JSON
  (JsonUtils + ERROR_INFO_TYPE), migration unchanged.
- python-backend: optimization_lifecycle builds {exception_type, message,
  traceback} from the caught exception; status_manager sends the structured dict
  (raw-REST fallback preserved for the pinned older SDK).
- SDK: rest_api optimization types + update param -> Optional[ErrorInfo];
  base_optimizer finalize builds the structured dict.
- FE: Optimization.error_info -> BaseTraceDataErrorInfo; RunErrorPanel prefers
  the structured error_info.message over log-scraping (falls back to logs).
Verified: Java OptimizationsResourceTest#updateById 3/0/0 (testcontainers),
python-backend unit 263 passed, SDK finalize 20 passed, FE tsc/eslint/vitest
green. e2e + fern regen deferred to CI.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: e2e reads structured error_info as dict (pinned SDK)
The syntax-error e2e accessed optimization.error_info.message, but the
python-backend's pinned (released) opik SDK returns error_info as a plain dict
(extra field) rather than a typed ErrorInfo object -> AttributeError. Read the
message/traceback via a helper that handles both a dict and a future typed
object. The structured error_info is persisted correctly; only the test's
access was wrong.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: address review — blank error message + validate ErrorInfo
- RunErrorPanel: an empty error_info.message no longer short-circuits the ??
  chain (blank panel); use ?.trim() || so it falls back to the log-derived
  message.
- OptimizationUpdate: mark errorInfo @Valid so the nested ErrorInfo @NotBlank
  constraints cascade on the update request body.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] [FE] style: prettier-wrap errorMessage useMemo in RunErrorPanel
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] address alexkuzmik review: metric-name idioms, aliased BaseMetric, old-SDK fallback
- process_worker._metric_name_ast: recover name from super().init(name="...")
  (the common no-name-param idiom) so the UI no longer shows "code" for it
- process_worker._find_basemetric_classdef: resolve aliased BaseMetric imports
  (from ... import BaseMetric as BM) so class X(BM) is detected at build time
- optimization.update(): on old pinned opik lacking the error_info kwarg, fall
  back to a raw REST PUT (mirrors worker status_manager) so the status/error
  transition still lands instead of throwing and stalling the run
- tests: super().init name idiom + aliased-import build
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] self-review fixes: submit-gate accuracy + consistent class selection
Code-review (full, workflow) surfaced three genuine correctness gaps in the
custom-code-metric submit gate and backend signature detection:
- optimizations.ts: default-less kwargs.get("x") no longer marks a column
  REQUIRED. .get() is missing-safe (returns None) and the editor helper copy
  recommends it as the safe accessor, so blocking submit contradicted our own
  guidance and blocked valid runs. Only kwargs["x"] subscripts gate submit.
- optimizations.ts/useOptimizationsNewFormHandlers.ts: the gate now also
  requires a strict score() signature's positional params (no **kwargs) via
  new extractRequiredScoreParams — an unmapped param with no same-named column
  would raise a missing-arg TypeError at runtime (silent all-zero run) that the
  kwargs-only scan missed.
- process_worker._find_basemetric_classdef: when a file declares multiple metric
  classes, pick the alphabetically-first (matching get_metric_class's
  inspect.getmembers order) so the statically-inferred score() signature flags
  match the class actually instantiated at scoring time.
Tests added for all three. Left as intentional/documented: no build-time exec
(security), dynamic metric-name -> "code" fallback, fabricated graceful-error
traceback (@NotBlank), output-precedence flip (LLM output correctly wins).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: rename error_info migration 000103 -> 000105 (main merged 000103/000104)
main added 000103_add_config_to_cipx_spends and 000104_add_plan_to_cipx_trace_identities
after this branch's 000103, so check-migration-prefix-conflicts failed. Bump ours to
the next free prefix and its changeset id to match.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] address Baz review: base-ctor name scan, **kwargs required params, ApiError contract
- process_worker._metric_name_ast: only read the name from the base constructor
  call (super().init or a declared-base init), not any incidental
  Helper.init(name=...) in the metric body (was misnaming objectives).
- optimizations.extractRequiredScoreParams: a trailing **kwargs no longer drops
  required declared positional params (kwargs absorbs only undeclared extras, so
  a missing declared param still raises TypeError). Return [] (defer to backend)
  when >1 score() def makes class-selection ambiguous. Parse-error stays fail-open
  (best-effort pre-check; backend AST is authoritative).
- optimization.update() fallback: translate a non-2xx raw-PUT response into the
  SDK's ApiError (status_code/headers/body) instead of leaking
  httpx.HTTPStatusError, preserving the error contract for .
Tests added for all three.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: FE metric-name extractor mirrors backend AST (objective_name parity)
Baz: the backend AST name extractor now recognizes super().init(name=...),
declared-base ctor calls, and class-level name=..., but the create path derived
objective_name from a FE regex that only matched the init param default. So
a metric using super().init(name="real") was stored as objective_name="code"
while its scores were emitted under "real" -> the v2 UI keyed by "code" and showed
"-". Broaden extractMetricNameFromPythonCode to mirror the backend precedence:
base constructor -> init param default -> class attribute (self.name guarded).
Also benefits the online-eval rule score-name path that shares this extractor.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] address review: cascade errorInfo validation, preserve on upsert, structure-aware FE name extractor
Backend (thiagohora findings):
- Optimization.errorInfo and updateOptimizationsById request param now carry
  @Valid so ErrorInfo's @NotBlank constraints cascade at the request boundary
  (mirrors studioConfig / the create+upsert endpoints).
- OptimizationService.upsert preserves an existing persisted failure reason when
  a re-upsert arrives with a null errorInfo, matching the studioConfig
  preservation branch (upsert is a full-row replace).
- OptimizationDAO.mapToDto guards error_info deserialization in try/catch-log-null
  like studio_config, so a malformed value degrades the field instead of failing
  the whole row.
- Added an errorInfo-only case to the updateById MethodSource, covering the
  branch that persists a failure reason without name/status.
Frontend (baz findings):
- extractMetricNameFromPythonCode now strips comments and triple-quoted
  docstrings before matching, so a decoy super().init(name=...) / name=...
  in a comment or docstring no longer injects a fake metric name.
- The class-level name = "..." fallback is scoped to the class-body
  indentation level, matching the backend AST (ast.Assign in cls.body) and
  excluding method-local assignments. Added regression tests.
Also renamed the error_info migration 000105 -> 000106 to resolve the prefix
conflict with main's 000105_add_id_at_to_spans.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] test: cover error_info preservation on SDK re-upsert
Adds an integration test that records a failure reason via the PATCH/update
path, then re-upserts the optimization with a null errorInfo and asserts the
persisted reason survives the full-row replace (the OptimizationService.upsert
preservation branch added for thiagohora's finding).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] address baz re-review: multi-class name extractor, malformed-source + clobber test coverage
- rules.ts extractClassBodyName now scans every class declaration (bounded to
  each class body), so a helper class declared before the metric class no longer
  causes the class-level name = "..." to be missed.
- FE tests: added a malformed-Python case (unterminated docstring -> null) and a
  helper-class-before-metric case.
- Backend test errorInfoPreservedOnReUpsert now also asserts studioConfig is not
  dropped by the full-row-replace re-upsert (status ordering is last_updated_at
  governed, so intentionally not asserted).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: FE metric-name extractor resolves the same class the backend instantiates
The previous multi-class extractor returned the first name = "..." found across
all class bodies, so a helper (or sibling) class could win and yield a metric name
that doesn't match the scored name — worse than null, because getFeedbackScore(...,
objective_name) then misses AND expectedMetricNames polls for a name that never
arrives (until MAX_REFETCH_TIME), rendering a phantom expected metric.

Now mirror the backend (process_worker._find_basemetric_classdef / get_metric_class):
- Collect BaseMetric aliases (literal + import ... as alias).
- Select the class whose declared bases include BaseMetric (name, dotted, or alias);
  when several, the alphabetically-first (Python min-by-name) — the same class the
  backend instantiates.
- Extract the name from ONLY that class body, in precedence order
  (super().init(name=) -> init default -> class-body-level name = "...").
- If no BaseMetric subclass is identifiable, return null and defer to the backend
  (which would reject non-BaseMetric code anyway) rather than guess a wrong name.
Added regression tests: helper-class-with-own-name doesn't win, alphabetical
selection among multiple metric classes, import-alias and dotted base recognition,
and null when no BaseMetric subclass is resolvable.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix: renumber optimizations error_info migration to 000107 (prefix conflict with main)
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
- [OPIK-7172] address review: multiline alias imports, PEP 695 headers, init-scoped name extraction; migration trailing newline
- rules.ts collectBaseMetricAliases handles parenthesized multiline imports
- class header regex accepts PEP 695 type parameters (class MT:)
- base-constructor name extraction scoped to the metric class's own init,
  accepting super().init and declared-base/alias init calls, ignoring
  helper-object init calls — mirrors process_worker._metric_name_ast
- 000107 migration ends with the required empty line
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
- [OPIK-7172] fix: metric-name extractor captures balanced constructor args
Address baz finding (rules.ts:262): the super().__init__(...) / base-ctor
argument capture used [^)]*, which stops at the first ). A nested call
before the name kwarg — e.g. super().__init__(config=make_cfg(), name="foo")
— was truncated, so name wasn't found and the extractor fell back to "code",
storing a wrong objective_name (getFeedbackScore misses, expectedMetricNames
polls until MAX_REFETCH_TIME).

Match only the ctor opening and capture the FULL balanced argument list (via a
new balancedArgs helper that respects nested parens + string literals), then
find the name kwarg in it. Added regression tests for a nested call before and
after the name kwarg.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] review: narrow mapToDto JSON catch to UncheckedIOException
Address thiagohora finding (OptimizationDAO:1006): the error_info (and adjacent
studio_config) deserialization caught bare Exception, masking unexpected runtime
failures (e.g. NPEs) alongside the expected JSON parse error. JsonUtils.readValue
throws UncheckedIOException on a malformed payload, so catch that narrowly and
let real bugs surface; the row-id context in the log is preserved.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: only inject output into code metrics that accept it
Address thiagohora finding (metrics.py:894): the LLM output was injected into
score()'s kwargs unconditionally. For a strict signature that doesn't declare an
output param (e.g. def score(self, reference)), that arrives as an
unexpected keyword -> TypeError -> swallowed to a masked ScoreResult(0.0) — the
exact OPIK-7172 failure mode the strict-signature handling was added to remove.

Inject output only when the signature can accept it: accepts_var_keyword or "output" in score_params. Added a regression test for a strict metric that does
not declare output.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] docs: correct arguments-map docstring to match code
Address thiagohora finding (metrics.py:799): the docstring said **kwargs metrics
receive the remaining columns "minus those consumed as a rename source", but the
code splats every column (data = {**dataset_item}) and only overlays the
renamed param — so a rename source IS still present under its original name. Also
refreshed the output bullet to match the now-conditional injection (a strict
signature without an output param does not receive it).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: keep traceback tail (not head) when truncating error_info
Address thiagohora finding (status_manager:94): message/traceback were truncated
with value[:MAX], keeping the HEAD. Python tracebacks put the innermost frame and
the actual raise site at the END, so for long tracebacks the persisted reason
showed only outer runner frames and cut off the frames closest to the failure —
undercutting OPIK-7172's whole point (surfacing the failure reason).

Now keep the message head (short, meaningful at its start) but the traceback
TAIL, with a "...[traceback truncated]..." marker, capped at MAX_ERROR_INFO_LENGTH.
The caller's dict is copied, never mutated.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: make update_status error_info/metadata keyword-only
Address thiagohora finding (status_manager:61): the merge inserted error_info
between status and metadata, changing the 2nd positional's meaning — a future
positional caller update_status("completed", some_metadata) would silently
bind some_metadata to error_info. All in-repo callers already use keywords, so
make error_info/metadata keyword-only (*) to remove the trap.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] test: cover error_info send / suppression / truncation
Address thiagohora finding (status_manager:89): the error_info path had no unit
coverage (only touched indirectly by e2e). Added TestErrorInfoForwarding:
- mark_error({...}) routes through the raw client with error_info in the body;
- mark_error(None)/mark_error({}) sends NO error_info (status-only, typed path)
  so it can't clobber a previously persisted reason;
- an over-length message is head-truncated and an over-length traceback keeps
  its tail (innermost frame) under the cap, without mutating the caller's dict.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: align build-time metric-class selection with runtime
Address thiagohora findings (process_worker _find_basemetric_classdef vs
get_metric_class, metrics.py:848):
- get_metric_class now restricts to classes DEFINED in the module
  (cls.__module__ == module.__name__), so an IMPORTED concrete BaseMetric
  subclass (e.g. from opik.evaluation.metrics import Equals) is never
  instantiated instead of the user's class just because its name sorts first (#6).
- _find_basemetric_classdef now resolves INDIRECT subclasses transitively over
  classes defined in the file, so it selects the same alphabetically-first class
  runtime issubclass picks — no more build/runtime divergence that applies the
  wrong signature/name (silent 0.0 / "-") (#2).
- validate_user_code no longer hard-rejects when the only BaseMetric link is
  through an IMPORTED base (statically unresolvable but instantiable at runtime);
  it defers with permissive defaults when a class is defined, and still rejects a
  file with no class at all (#1 regression guard).
Added tests: transitive indirect-subclass selection matches runtime, imported
subclass is not instantiated, imported-base defers instead of rejecting, no-class
still rejected.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] fix: FE metric-class resolver follows indirect subclass chains
Keep the FE objective_name extractor in sync with the backend selector change:
findMetricClassBody now resolves INDIRECT (transitive) BaseMetric subclasses
defined in the file — collecting all class declarations, seeding from direct
alias subclasses, then transitively adding classes that subclass an
already-known metric class — before picking the alphabetically-first. This
matches process_worker._find_basemetric_classdef / get_metric_class, so an
indirect subclass that runtime instantiates yields the same objective_name here
(no "-"/stalled-polling mismatch). Unresolvable (imported-base) cases still
return null and defer to the backend. Added a transitive-subclass test.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] review: read real signature in imported-base fallback + build/score test
Address baz follow-ups on the review batch:
- process_worker.validate_user_code: when no class statically resolves to a
  BaseMetric subclass but classes are defined, read name + signature flags from
  the alphabetically-first defined class that declares score() (mirrors runtime
  get_metric_class), instead of returning blanket permissive defaults. This keeps
  a STRICT imported-base metric from being force-splatted into a masked 0.0 and
  preserves its objective_name. Only when no defined class declares score() (it's
  inherited from the imported base) do we fall back to permissive defaults.
- Strengthened tests: the imported-base case now builds via MetricFactory.build
  and asserts the real ScoreResult value/name (Equals-based), plus a direct
  validate_user_code assertion that the strict signature is read (not blanket).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] review: restrict metric-class selection to top-level classes
Address baz finding (process_worker:362): _find_basemetric_classdef and the
imported-base fallback collected ClassDefs via ast.walk, which includes classes
NESTED in a method/class. Those never become module attributes, so runtime
get_metric_class (inspect.getmembers) never sees them — but a nested helper with
a score() sorting alphabetically first could be picked at build, disagreeing
with runtime. Both now use _top_level_classdefs (tree.body only), matching
runtime's module-level view. Added a regression test: a nested AHelper.score()
must not override the top-level metric's strict signature.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- [OPIK-7172] chore: bump error_info migration 000107 -> 000108 (prefix conflict)
main merged 000107_apply_traces_local_v2_real_data_codec_refinements; rename the
optimizations error_info analytics migration to the next free prefix (000108) and
update its changeset id to clear the migration-prefix-conflict check.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Co-authored-by: Claude Opus 4.8 (1M context) noreply@anthropic.com
Co-authored-by: Yaroslav Boiko yaroslavb@comet.com
下载附件
- 源代码 (ZIP)
- 源代码 (TAR.GZ)

0 发布 8,340 Git 标签

[OPIK-7172] [BE][FE][SDK] feat: re-expose custom-code metric in Optimization Studio (#7460)