[OPIK-7354] [BE] [FE] fix: accept flat judge output and stop escaping judge prompts

[miguelgrc]

Details

LLM-as-judge online-evaluation rules returned scores on some traces and {"scores": []} on others, non-deterministically, for the same rule and template. The judge was given two contradictory output-format instructions in one prompt — the built-in template body asked for a flat {"score": ...} object while the backend appended a nested, schema-named one — and only the nested shape parsed. Which one the model followed was effectively a coin flip.

• toFeedbackScores now takes the rule's schema and also accepts a flat single-score object, so rules created before this fix keep scoring without being re-created. Schemas with several scores still parse nothing rather than guess which score a nameless object belongs to.
• Score values the judge quoted are parsed instead of silently stored as 0. JsonNode.decimalValue() answers ZERO for every non-numeric node, so "score": "0.8" was recorded as 0.0 — a wrong score rather than a missing one, indistinguishable from a genuine zero. Values that are neither a number nor a boolean are now reported instead of stored.
• Array reasons are joined rather than dropped (asText() on an array yields "").
• A judge answer that cannot be read is surfaced on the rule's logs page, instead of the run looking successful with no score and no explanation.
• MustacheParser no longer HTML-escapes substituted values, so a trace's JSON reaches the judge as written rather than as "-escaped text. render() and renderUnescaped() collapse into a single non-escaping render(), matching the frontend preview and PythonTemplateParser. This also repairs multimodal URLs assembled from template variables on the scoring path, which never went through the unescape shim: escaping rewrote & and = inside substituted values, so a base64 data URI lost its = padding to =. URLs written literally in a template were unaffected, since Mustache escapes substituted values only.
• Built-in templates (frontend): removed the contradicting output-format blocks from Meaning Match and Structured Output Compliance, delimited Meaning Match's item-to-score from its few-shot examples, removed the field-name instruction from AnswerRelevance, and declared the 0.0–1.0 answer-relevance scores as DOUBLE instead of INTEGER (an integer constraint collapses them to 0 or 1 on providers using native JSON-schema output).

Review rounds

a100d5d — judge-answer diagnostics:

• The judge's raw answer no longer reaches the rule's logs. NOT_JSON / NOT_A_JSON_OBJECT report its size instead; the answer itself stays in the server log. The answer echoes the scored trace's content and these logs are persisted per workspace, so this follows "Never Log" in .agents/skills/opik-backend/SKILL.md and AgenticScoringService.summarizeResponse, which reports shape rather than text for the same reason. NO_SCORE_FIELDS still reports the field names: they are structural, and they are the diagnostic that actually identifies a mismatch.
• The reported field names are capped. They come from the judge's answer, so a reply with thousands of top-level fields would otherwise write one very large row.
• toParsed() copies its lists, so a caller cannot mutate a parsed result through the accumulator's own lists.
• Internal logs are key='value', and no longer repeat a static "expected structure" paragraph on every occurrence. The user-facing messages stay prose — they render on the rule's Logs page for a customer, not in ops telemetry.

40e08c5 — judge-answer handling:

• The nested pass only accepts score names the rule declares, matched case-insensitively. Previously any nested object with a score field could claim the score name, so a judge that wrapped its answer in an extra key ({"details": {"score": 0.4}}) both stored a score under a name the rule never declared and suppressed the flat fallback.
• Textual booleans are read through BooleanUtils.toBooleanObject, covering true/false, yes/no, y/n, t/f, on/off, plus pass/fail. These previously produced no score at all.
• scoreNameMapping is applied before the log calls, so warnings name the score the user configured rather than the internal assertion_N key.
• Both unreadable-answer paths log at WARN without a stack trace, using JsonProcessingException.getOriginalMessage() so the raw answer isn't echoed through the exception message.
• Built-in templates: Meaning Match's few-shot exemplars are JSON again, in the nested schema-named shape the parser accepts; User frustration declares DOUBLE.

96d880c — translating, sanitising and rounding what the judge sends:

• Scores with more precision than ValidationUtils.SCALE are rounded with RoundingMode.HALF_UP rather than truncated by ClickHouse on insert, and values outside MIN/MAX_FEEDBACK_SCORE_VALUE are rejected up front instead of failing the whole batch with Code 69 ARGUMENT_OUT_OF_BOUND. Narrower than it first appears: an unquoted JSON number large enough to lose precision is already a double by the time Jackson hands it over, so the scale path is only reached for quoted values and small magnitudes.
• Parse problems are carried structurally (ResponseProblem: kind, evidence, offending field names) instead of as pre-rendered strings, and a single renderer produces both the internal log and the user-facing message so they cannot drift.
• Reported field names are quoted and stripped of control characters, so judge-supplied text cannot inject line breaks and forge extra lines in the persisted logs.

25bbc6e — Gemini content conversion:

• OpikGeminiChatModel.convertMessage rebuilt a message's content list from only TextContent, ImageContent and the converted VideoContent, so any other content type silently fell off the list despite a comment claiming otherwise. It now special-cases VideoContent and passes everything else through. This only ever affected audio or file content arriving alongside video, since the rebuild is skipped entirely for messages without video.

5e6715e — the new test fixture is built through LlmAsJudgeOutputSchema.builder(), per .agents/skills/opik-backend/SKILL.md.

77711b52 — determinism and proportion in what gets reported:

• A judge answer can claim one declared score under several case-variant keys ({"Score": ..., "score": ...}). Both resolved to the same canonical name and both were stored; insertFeedbackScores binds the list without deduping, and feedback_scores is a ReplacingMergeTree keyed on (workspace_id, project_id, entity_type, entity_id, author, name) versioned by last_updated_at, which is a now64(9) column default the insert never sets. So the two rows collapsed to whichever won on a timestamp the application does not control — a nondeterministic score. The answer's first occurrence now wins.
• Reported field names are capped where they are collected rather than where they are rendered. Previously the whole list was materialised, stored on ResponseProblem and re-mapped by withUserFacingNames, with only the display capped. The omitted count is now carried alongside, so the message is unchanged while the retained list is bounded.
• \p{Cntrl} is hoisted to a static final Pattern; it is applied inside a per-name map, so it was being recompiled per element.
• The parsing-test fixtures go through LlmAsJudgeOutputSchema.builder(), matching the sibling change above.

1421bd46 — locale-independent matching, and proportionate diagnostics:

• Score-name and verdict lookups lowercased with the default locale, which nothing in the image pins. Under a Turkish or Azeri locale "FAIL" lowercases to "faıl" and stops mapping to false, and case-insensitive score-name matching breaks for any name containing an I. All three sites now use Locale.ROOT. This does not fire on the shipped image (no LANG/LC_ALL, no -Duser.language, so the default is en/C) — it is taken because the lowercasing is a lookup key rather than display text.
• The unusable-value warning now reads "Could not use the score value … expected a boolean or a number between and ". An out-of-range number is rejected on the same path as an unreadable one, so "could not read" was wrong for it; the bounds come from ValidationUtils rather than literals.
• ResponseProblem and ParsedFeedbackScores carry @Builder(toBuilder = true), the former with @NonNull components, and the rename path rebuilds through toBuilder() instead of re-listing every component positionally.

c5353e58 — reporting what was discarded, and keeping the answer out of the logs:

• A judge answer can carry a score under a name the rule never declares. It was dropped at DEBUG while the declared scores parsed, so the run looked successful and the user never learned a score had been discarded — the same silent-nothing-happened shape this PR exists to remove, introduced by the declared-name restriction added earlier in this branch. Those names are now carried on the parsed result and reported on the rule's Logs page. foundNothing() deliberately still ignores them, so an answer of only undeclared scores continues to reach the flat fallback and then NO_SCORE_FIELDS.
• The three parse-failure warnings no longer echo the judge's answer. This branch had introduced that logging; origin/main logged none of it. Size, the Jackson error and the structural field names remain. This matches AgenticScoringService.summarizeResponse, which the scorer already used to report a response's length and shape rather than its text.
• Both diagnostic records snapshot the lists they are handed, and an unset list reads as empty rather than null. The internal construction sites name their fields through the builder rather than passing three same-typed List<String> positionally, where a swap would compile silently and mislabel every diagnostic.

37ee8357 — a blank declared name must not reach storage:

• A rule can be saved with a blank score name, since LlmAsJudgeOutputSchema.name is @NotNull rather than @NotBlank, and an item built in code skips bean validation. The flat fallback added by this PR copied that name onto a stored score, where name is part of the primary key. accept() now rejects it, covering the nested path too. Fixing it at the API boundary instead was tried and backed out: @NotBlank is inert without @Valid on the schema list, and adding that also activates the dormant @NotNull on type and description, changing create/update validation for existing callers — wider than this PR, and tracked separately.
• logUnreadableResponse is now logResponseIssues: it reports undeclared scores as well as unreadable answers, so the old name had stopped being accurate.
• The rendered field-name list is logged as one double-quoted value. It carries commas and single quotes, so an unquoted placeholder left the internal line ambiguous to a key='value' reader.

56dd6063 — keep scoring rules the name restriction had silenced:

• The built-in Meaning Match template told the judge to answer under a literal {score_name} key. Nothing substitutes single-brace placeholders — Mustache expands {{var}} — so on providers that treat the output format as advice the model saw two contradictory key names and the rule recorded nothing. All three copies now use the declared name; the placeholder belongs only to the custom template, where the user replaces it by hand. Introduced by the earlier commit in this branch that replaced the prose exemplars.
• Restricting stored scores to declared names stopped scoring rules whose prompt asks the judge for a different name (relevance_score against a schema entry named Relevance), which stored scores before this branch. A third attribution pass now handles that when the schema declares exactly one score and the answer carries exactly one score-bearing object. It runs after the flat pass, so a stray nested object still cannot outrank a flat score. Attribution is by arity, so an invented name on a single-score schema is now attributed rather than reported — the trade being that one plausible score beats none on a background path.
• An empty schema (@NotNull, not @NotEmpty) discarded every score the judge returned and reported that the answer had none of the expected fields, which was untrue. With nothing declared there is nothing to match against, and nothing that could be hijacked, so the judge's own names are kept.
• The Gemini conversion read Video.url() unconditionally, but a Video carries either a url or inline base64: MinIO-staged attachments arrive as base64, so this threw and, had it not thrown, would have dropped the payload. Pre-existing, but in the method this branch rewrote and the same silently-lose-input class.
• The parse flow is expressed as ordered passes on the accumulator that owns them, rather than free functions handed a mutable parameter.

Change checklist

• User facing
• Documentation update

Issues

• OPIK-7354

AI-WATERMARK

AI-WATERMARK: yes

• Tools: Claude Code
• Model(s): Claude Opus 5
• Scope: full implementation, including tests
• Human verification: interactive review of every change with the author, iterative design review, an automated code review pass whose findings were verified and applied, and PR review feedback triaged with each item either fixed or answered on the thread

Testing

Commands run locally (macOS; Maven with Docker testcontainers for the container-backed suites, Node for the frontend):

• mvn -o test -Dtest=OnlineScoringEngineParsingTest — 64 tests, green, no containers. All parsing and reporting behaviour.
• mvn -o test -Dtest=GeminiVideoSupportTest — 11 tests, green, no containers.
• mvn -o test -Dtest=MustacheParserTest,ExperimentItemProcessorTest,ExperimentMessageRendererTest,TestSuiteEvaluatorMapperTest,TestSuiteAssertionSamplerTest,PythonTemplateParserTest — 80 tests, green.
• mvn -o test -Dtest=OnlineScoringEngineTest — 121 tests, green, before the parsing tests were split out into their own class. Not re-run since the split (see "Not run" below).
• mvn -o test-compile — green.
• npm run lint && npm run typecheck (frontend) — green.
• pre-commit on the changed files — spotless, eslint and frontend typecheck all green.

The parsing tests were moved out of OnlineScoringEngineTest into OnlineScoringEngineParsingTest: toFeedbackScores and logUnreadableResponse are pure static functions, but that class starts MySQL, ClickHouse and Zookeeper for its end-to-end tests, so asserting that a quoted score parses previously required a Docker daemon and ~45s. The end-to-end Redis-flow test stays behind, because it genuinely needs the containers.

Scenarios validated by new tests:

• Flat single-score reply is scored under the schema's name; nested reply unchanged; flat reply against a multi-score schema still scores nothing.
• A nested object whose key the rule never declared is reported rather than stored, and does not suppress the flat fallback.
• Flat "score": null is reported as skipped rather than dropped.
• Quoted numbers and booleans ("0.8", "1", "true", "TRUE", " false ", "yes", "pass") are parsed; unusable values ("high", "", [], {}) are reported instead of stored as 0.
• Values outside the allowed range are rejected while sibling scores still store; excess-precision values are rounded rather than truncated.
• Sibling scores that parse are kept when one score's value is unusable.
• Answers that are not JSON, are JSON but not an object, or carry none of the expected score fields each produce a distinct reported reason; an empty object names its absent fields rather than rendering a blank.
• A judge answer that is not JSON is reported by size only — asserted not to contain an email or token embedded in that answer.
• Field names are capped with the remainder counted, and control characters in them are neutralised, so one reply cannot produce an unbounded or forged log row.
• The user-facing warning fires for unusable values and unreadable answers, and stays silent when the answer was fully readable.
• Rendering no longer escapes & < > ' " = ` in substituted values, and keeps substituted JSON structurally intact.
• Audio content survives Gemini's video-to-image conversion when both are present in one message. Verified to fail against the old loop (Expected size: 2 but was: 1).
• Two case-variant keys claiming one declared score yield a single score, taking the answer's first occurrence. Verified to fail without the guard (Expected size: 1 but was: 2).
• Affirmative and negative verdict spellings including uppercase PASS/TRUE/FAIL/FAILED, which exercise the pass-fail map rather than BooleanUtils.
• An answer mixing a declared and an undeclared score stores the declared one, reports the undeclared one by name, and records no whole-answer problem.
• A rule declaring a blank score name ("" or whitespace) stores nothing rather than writing a blank name into the score table. Verified to fail without the guard.
• A renamed single score is attributed to the declared name; several undeclared candidates are reported rather than guessed between; an invented name on a multi-score schema is still reported; an empty schema keeps the judge's own names.
• Inline base64 video survives the Gemini conversion. Verified to fail without the guard (Cannot invoke "java.net.URI.toString()" because ... Video.url() is null).
• An end-to-end pass through the real Redis producer/consumer flow — project and rule created over the API, trace event published, LLM mocked to return the flat shape — asserts the score reaches the feedback-score batch under the schema's name. Verified to fail without the fix (Expected size: 1 but was: 0 in: [], matching the reported symptom) and pass with it.

Verified against a deployed PR environment (pr-7758) with a script driving the API end to end — fresh project, rules created before the traces they score, then polling for stored scores and reading the rule's Logs page. Ten flows, all passing, on both provider families: OpenAI (native JSON-schema output, ToolCallingStrategy) and Anthropic (appended text instruction, InstructionStrategy). Covered: trace/span/thread scorers, nested and flat prompts, non-escaped rendering, DOUBLE vs INTEGER, excess-precision rounding against real ClickHouse, and an out-of-range score rejected while its sibling stored with the warning surfacing to the user.

The built-in Meaning Match template was additionally exercised by hand, since it reaches the backend only through the rule-creation form: a rule created from the template in the UI against Anthropic scored three traces correctly — true for a reworded match, false for a wrong painter, true for "seven continents" against "7" — with no warnings on the rule's log. The judge's reasons came back echoing the template's own exemplar wording. Every scripted run used custom prompts, so this path had not been covered before.

That run also confirmed empirically what the INTEGER to DOUBLE template change is for: told to emit 0.75, a DOUBLE schema stores 0.75 on both providers, while an INTEGER schema stores 0.0 on OpenAI (the JSON schema constrains the model, which floors) and 0.75 on Anthropic (the type is only advisory). The pre-fix templates therefore scored differently per provider.

Not run:

• OnlineScoringEngineTest has not been re-run since the parsing tests were split out of it. The edit there was the deletion of one contiguous block and the class compiles, but its end-to-end tests have not executed against the final state — CI covers this.
• The full backend suite (mvn test) was attempted locally and aborted: too many concurrent testcontainers for one machine, so unrelated classes failed with ContainerLaunchException on ClickHouse and MySQL startup. An environment limit, not a code failure — the same classes pass when run targeted. CI is the real signal.
• No verification against a live LLM provider. The claim that multimodal URLs are repaired is derived from the code path plus a reproduction of the escaping behaviour, not from a provider round trip. The Gemini audio fix is likewise verified by unit test only — whether the provider accepts AudioContent is unchanged by this PR, since audio-only messages already reached it untouched.

Documentation

meaning_match.mdx (both docs and docs-v2) is updated to match the revised built-in template: the documented output format and few-shot examples showed the flat shape this PR removes, so leaving them would have documented a format the parser no longer receives from the template. The other metric pages describe the Python SDK metrics rather than the built-in prompt text, so they remain accurate and are unchanged.

[github-actions]

⏱️ pre-commit per-hook timing

Hook	Description	Result	Duration
🌐 typecheck — frontend	Whole-project tsc type check	✅	41.95s
☕ spotless — java backend	Format Java code	✅	4.00s
🌐 eslint — frontend	Lint + autofix JS/TS	✅	2.27s
Total (3 ran)			48.22s

⏭️ 39 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	⏭️
🧪 pre-commit wrapper smoke tests	Self-test the wrapper scripts	⏭️
📘 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	⏭️

[baz-reviewer]

Unbounded error-path response accumulation

The no-score path calls toList() on every top-level field before formatting the warning, so large judge responses trigger an unbounded diagnostic allocation and log message on the synchronous scoring path — should we cap the field names and report omitted fields before logging and returning NO_SCORE_FIELDS?

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/main/java/com/comet/opik/api/resources/v1/events/OnlineScoringEngine.java
around lines 1132-1141 inside `toFeedbackScores`, the code builds `topLevelKeys` using a
stream and `.toList()` over all `structuredResponse.fieldNames()` just to log them and
set `ResponseProblem.Kind.NO_SCORE_FIELDS`. Refactor this to avoid unbounded allocation:
collect only a fixed maximum number of field names (e.g., same cap as
MAX_REPORTED_RESPONSE_CHARS or a small constant), track how many additional keys
exist/are omitted, and log an abbreviated representation like "first N keys: ... (and M
more)" while setting evidence similarly. Ensure the synchronous scoring path never
formats/allocates the full list and that the warning message remains bounded in size.

[miguelgrc]

Fixed in 77711b5 — correct, and the earlier round only addressed the display. The full field list was still materialised, stored on ResponseProblem and re-mapped by withUserFacingNames; only the rendering was capped.

The cap now sits where the names are collected (.limit(MAX_REPORTED_FIELD_NAMES)), with the omitted count carried alongside so the message stays truthful — the total comes from ObjectNode.size(), which is O(1). The user-visible text is byte-identical; the existing assertion on it was kept to prove that, and only the retained-list assertion changed (hasSize(500) → hasSize(10) plus omittedFields == 490).

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Commit 77711b5 addressed this comment by limiting collected field names to MAX_REPORTED_FIELD_NAMES before toList(), tracking omitted fields, and rendering the bounded list with an omission count.

[baz-reviewer]

Empty judge replies fail scoring

extractJson passes empty replies to ObjectMapper.readTree(String), which returns Java null, so structuredResponse.isObject() throws before producing ParsedFeedbackScores.problem(...) or calling logUnreadableResponse — should we treat a null tree as an unreadable response before dereferencing it?

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/main/java/com/comet/opik/api/resources/v1/events/OnlineScoringEngine.java
around lines 1107-1114 inside the `toFeedbackScores(ChatResponse chatResponse,
List<LlmAsJudgeOutputSchema> schema)` method, add a null/empty guard after
`OBJECT_MAPPER.readTree(content)` so you never call `structuredResponse.isObject()` when
`structuredResponse` is null (e.g., when `extractJson` yields "" or whitespace). If the
parsed node is null (or the input `content` is blank), immediately return
`ParsedFeedbackScores.problem(...)` using the same classification used for invalid JSON
(likely `ResponseProblem.Kind.NOT_JSON`) rather than letting the exception escape. Keep
the existing log+return behavior for real JSON parsing failures, but ensure the
null-tree path is handled explicitly before dereferencing.

[miguelgrc]

Skipping — this doesn't reproduce on the Jackson version in use. readTree("") returns a MissingNode, not Java null, so isObject() is safely false and the empty case already returns a NOT_A_JSON_OBJECT problem.

Two pieces of evidence: feedbackParsingArguments already includes an empty-string case expecting zero scores, and it passes — a null tree would make it error on the dereference, not pass. And the observed log for an empty reply is Judge answer was not a JSON object: size='0 chars' response='', which only reaches that branch via a non-null node.

readTree returning null for empty input was older Jackson behaviour.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Thanks for the detailed correction—MissingNode makes the dereference safe here. I’ll save this to memory once the PR is merged.

[baz-reviewer]

Noisy error telemetry for judge responses

Handled JsonProcessingException parse failures are logged at ERROR with a full stack trace even though they return NOT_JSON, so expected unreadable responses create noisy logs that obscure real backend failures; should we log NOT_JSON and NOT_A_JSON_OBJECT at WARN without the throwable, reserving stack traces for unexpected system failures?

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/main/java/com/comet/opik/api/resources/v1/events/OnlineScoringEngine.java
around lines 1117-1119 inside `toFeedbackScores(...)`, the catch for
`JsonProcessingException` logs at `ERROR` with the full throwable stack trace even
though this is a handled/unreadable-judge-output fallback that returns
`ResponseProblem.Kind.NOT_JSON`. Change this logging to `WARN` and log only the
actionable context (size and truncated response), without passing `e`/omitting the stack
trace. Also update the adjacent `NOT_A_JSON_OBJECT` branch (around lines 1112-1114 in
the same method) from `INFO` to `WARN` so unreadable judge answers use a consistent,
actionable severity.

[baz-reviewer]

Commit 40e08c5 addressed this comment by changing both handled parse-failure branches to WARN and removing the throwable stack trace from invalid JSON logging.

[miguelgrc]

Fixed in 40e08c5 — both unreadable-response paths now log at WARN, and the JsonProcessingException case no longer passes the throwable:

WARN  Judge answer was not valid JSON: size='35 chars'
      error='Unrecognized token 'Sure': was expecting (JSON String, Number …)'
      response='Sure! Let me evaluate that for you.'

Jackson's reason is extracted via getOriginalMessage(), so the diagnostic survives without a stack trace. NOT_A_JSON_OBJECT moved from INFO to WARN too, so the same class of outcome reports at the same severity.

Agreed on the reasoning: a judge answering in prose is an expected outcome this PR deliberately surfaces, so it shouldn't sit in the same bucket as system failures.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

testToFeedbackScores only asserts scores() size, so malformed, empty, or ambiguous inputs can pass even when the parser omits the appropriate ResponseProblem; the if (expectedSize > 0) branch also mixes two different assertion flows (full per-score checks vs. size-only checks) into one test, hiding which checks apply to each case. Should we split the valid-JSON case into its own dedicated test with the full relevance/conciseness/technical-accuracy assertions, and turn the parameterized test into a size-and-problem-kind check for the zero-score cases — extending feedbackParsingArguments() to also supply an expected ResponseProblem.Kind (or null for the valid case) and asserting parsed.problem() accordingly?

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/OnlineScoringEngineParsingTest.java:
1. Split the current single parameterized test into two: a dedicated test for the
valid/readable JSON case that keeps the existing
relevance/conciseness/technical-accuracy assertions, and a parameterized test covering
only the malformed/empty/ambiguous "size-only" scenarios. 2. For the parameterized test,
change `feedbackParsingArguments()` to return tuples of (aiMessage, expectedSize,
expectedProblemKind), where each unusable/ambiguous case expects the appropriate
`OnlineScoringEngine.ResponseProblem.Kind` (e.g., NOT_JSON/NO_SCORE_FIELDS as dictated
by the input). 3. Update the parameterized test body to parse once, assert `scores()`
size, and assert `parsed.problem()` is null when expectedProblemKind is null, otherwise
assert `parsed.problem().kind()` equals the expected kind, removing the `if
(expectedSize > 0)` branching entirely.

[miguelgrc]

Skipping — the coverage this asks for already exists, as separate tests rather than as extra columns on the parameterized one.

OnlineScoringEngineParsingTest now asserts the problem kind explicitly for each shape: whenResponseIsNotJson_thenReportsItAsUnreadable (NOT_JSON), whenResponseIsJsonButNotAnObject_thenReportsIt (NOT_A_JSON_OBJECT), whenResponseHasNoScoreField_thenReportsTheTopLevelKeys and whenResponseIsAnEmptyObject_thenNamesTheAbsentFields (NO_SCORE_FIELDS), plus whenScoreValueIsUnusable_... for the per-score case. So restructuring testToFeedbackScores would move assertions rather than add them.

The point about if (expectedSize > 0) mixing two assertion flows is fair as a readability matter — noting it rather than reshaping a pre-existing test in this PR.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Thanks for the clarification—separate tests already cover those failure shapes, so restructuring would not add coverage. I’ll save this context to memory once the PR is merged.

[thiagohora]

Reviewed the parsing/escaping changes across the backend parser, Mustache renderer, and the built-in frontend templates. The direction is good — the diagnostics and the flat-score fallback are real improvements. Six inline comments below, each verified against the branch.

The two I'd treat as blocking:

• MustacheParser.render() — dropping HTML escaping leaves the matching unescapeHtml4 on media URLs in OnlineScoringEngine (lines 844/851/858), so those URLs now get mangled rather than restored.
• toFeedbackScores flat fallback — gating on foundNothing() lets an unrelated nested object carrying a score key win the nested pass, so the trace gets a wrongly-named score and the real one is dropped, with no warning surfaced.

The rest are a user-facing warning that reports internal assertion_N keys, a boolean-parsing narrowing that drops "yes"/"no", and two built-in template issues (User frustration still INTEGER; Meaning Match exemplars no longer JSON).

Three lower-severity items I did not comment inline: the unbounded new BigDecimal(text) in toScoreValue (a >9.2e9 score would fail the whole scoreBatchOfTraces insert, not just that score); the Fern pages for answer_relevance/meaning_match still documenting the removed output-format blocks including answer_relevance_score; and the fixed 300 ms Mono.delay in testRedisFlowScoresTraceWhenJudgeAnswersFlat, where the same class already uses Awaitility.await().untilAsserted(...) (lines 473, 643).

[thiagohora]

Escape/unescape pair is now asymmetric.

Dropping HTML escaping here removes only one half of a matched pair — OnlineScoringEngine.buildUserMessageFromContentParts still calls StringEscapeUtils.unescapeHtml4 on every rendered image/video/audio URL (lines 844, 851, 858). That unescape now runs against raw, never-escaped strings.

Concretely: a multimodal judge rule renders an image_url whose value contains an entity-looking sequence, e.g. https://cdn.example.com/i.png?id=1®v=2. Previously Mustache escaped the & and unescapeHtml4 restored the original exactly; now the URL sent to the provider becomes ...?id=1®v=2, so the judge scores against a 404/wrong asset or the provider rejects the request.

The unescapeHtml4 calls should go away together with the escaping.

[miguelgrc]

Fixed in 40e08c5 — removed all three unescapeHtml4 calls, so the pair is symmetric again.

You're right that this was the half I missed. I had checked MessageContentNormalizer and concluded the scoring path never unescaped, without noticing the three calls in the file I was editing. Verified your example: escaping used to turn & into & and these calls turned it back, so the round trip was lossless; with the escaping gone they were "restoring" never-escaped text and ?id=1®v=2 became ?id=1®v=2 → ®.

Worth noting the PR description previously claimed this change repaired multimodal URLs. That claim was wrong for exactly this reason and has been corrected.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Commit 40e08c5 addressed this comment by removing all three unescapeHtml4 calls and passing rendered multimedia URLs directly to the content builders.

[thiagohora]

A stray nested object can hijack the score name and silently suppress the flat fallback.

The fallback is gated on collected.foundNothing(), but the nested pass accepts any top-level property whose value is an object carrying a score key — it never checks the name against the rule's schema.

For a single-score rule (schema ["Meaning Match"]) with an answer like:

{"score": true, "reason": ["matches"], "details": {"score": 0.4, "reason": "partial overlap"}}

the nested pass skips score/reason (not objects) but accepts details, so foundNothing() is false here and collectFlatSingleScore never runs. The trace gets a feedback score literally named details = 0.4 — nothing downstream filters names against the schema (OnlineScoringLlmAsJudgeScorer.evaluate only applies scoreNameMapping) — while the intended Meaning Match = 1 is dropped. No warning is surfaced either, since problem() is null and unreadableScoreNames is empty.

Worth restricting the nested pass to names present in the rule schema, or running the flat fallback whenever the top level has a score key rather than only when nothing at all was collected.

[miguelgrc]

Fixed in 40e08c5 — the nested pass now only accepts names the rule declares, matched case-insensitively and stored under the schema's own spelling.

Both halves of what you describe are covered: {"score": true, "details": {"score": 0.4}} no longer stores a score called details, and because that pass now recognises nothing, the flat fallback runs and Meaning Match = 1 is recorded. A name the rule doesn't declare becomes a reported NO_SCORE_FIELDS problem naming the keys the judge actually sent, rather than silent junk data.

I took the "restrict to schema names" option over "run the fallback whenever there's a top-level score" because it also closes the pre-existing half — a judge inventing a name used to get that name persisted as a real score, which no downstream filter caught.

Test: whenAnUnrelatedNestedObjectCarriesAScore_thenTheFlatScoreStillWins, plus cases for an invented name and a case-different one.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Commit 40e08c5 addressed this comment by restricting nested score collection to case-insensitive names declared in the rule schema and storing the schema’s spelling. Unrecognized nested objects are ignored, allowing the flat single-score fallback to record the intended score.

[thiagohora]

Common textual booleans now produce no score at all.

The unconditional decimalValue() fallback is replaced by a whitelist: boolean node, number node, text equal to "true"/"false", or BigDecimal-parsable text. That omits the spellings judges most often emit for boolean metrics — "yes"/"no", "pass"/"fail", "Y"/"N".

A BOOLEAN rule (e.g. the built-in Meaning Match) on Anthropic/Ollama via InstructionStrategy answering {"Meaning Match": {"score": "yes", "reason": "same fact"}} now returns empty from toScoreValue, lands in unreadableScoreNames, and writes no row to feedback_scores — a gap in the rule's metric chart where a value was previously always recorded.

Suggest widening the textual mapping to the usual affirmative/negative synonyms before falling through to unreadable.

[miguelgrc]

Fixed in 40e08c5 — via BooleanUtils.toBooleanObject, which covers true/false, yes/no, y/n, t/f and on/off, plus a four-entry map for pass/passed/fail/failed which it doesn't handle.

Accurate diagnosis of a regression I introduced: before my earlier commit decimalValue() returned 0 for "yes" — wrong, but a value existed; after it, nothing was recorded at all. Reusing commons-lang3 rather than hand-rolling the list means we also pick up on/off and t/f.

Tests: parameterized over both word sets.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Commit 40e08c5 addressed this comment by mapping common boolean spellings through BooleanUtils.toBooleanObject and adding support for pass/passed/fail/failed. These values now produce storable 0/1 scores instead of being marked unreadable.

[thiagohora]

The new warning reports internal keys instead of the user's names.

logUnreadableResponse is called with the raw parsed result here, before scoreNameMapping is applied at lines 387-388.

On the test-suite path (TestSuiteAssertionSampler → TestSuiteEvaluatorMapper.renameSchemaToAssertionKeys) every schema name is rewritten to assertion_1, assertion_2, … with the user's assertion text held in scoreNameMapping. So when a judge quotes a score value, the rule's Logs page shows "Could not read the score value for 'assertion_2' on traceId '…'" — a name that appears nowhere in the user's test suite. The message added to make this diagnosable doesn't say which assertion broke.

Applying scoreNameMapping before the log call (or passing it in) would fix it.

[miguelgrc]

Fixed in 40e08c5 — scoreNameMapping is now applied before the log calls, via a new ParsedFeedbackScores.withUserFacingNames(mapping).

This turned out to be purely an ordering bug: the rename already existed inline in the .map() below, so it ran after logging. Moving it above the log calls means one re-key now feeds both the logs and the stored scores, and the inline version is gone — so the two can no longer disagree.

One thing your comment doesn't mention but is worth recording: the pre-existing logSkippedNullScores had the same flaw, so the "skipped null score" message has always named assertion_N too. Both are fixed by the same change, since it re-keys nullScoreNames and unreadableScoreNames as well as the scores.

Test: whenAMappingIsGiven_thenNamesAreRekeyed.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Commit 40e08c5 addressed this comment by applying scoreNameMapping before logging, ensuring both unreadable and skipped-null score logs use user-facing names.

[thiagohora]

The sibling thread template has the same contradiction and wasn't fixed.

This INTEGER → DOUBLE fix is right, but "User frustration" at line 944 still has type: LLM_SCHEMA_TYPE.INTEGER while its prompt asks for "a frustration score as a decimal value between 0.0 and 1.0" with graded bands (0.8-0.9, 0.6-0.7, …) and an example of "score": 1.0.

On a structured-output provider (OpenAI/Gemini/OpenRouter/VertexAI) ToolCallingStrategy builds a JsonIntegerSchema for the score field, so the model is forced to emit an integer — every frustration score lands as 0 or 1 and the gradations the prompt asks for are unreachable. On non-structured providers InstructionStrategy renders the example as "score": 1 while the prompt body says decimal, which is exactly the contradictory-instruction case OPIK-7354 is about.

Same one-line change applies there.

[miguelgrc]

Fixed in 40e08c5 — User frustration now declares DOUBLE.

Confirmed the mechanism you describe: ToolCallingStrategy builds a JsonIntegerSchema for the score field, so on a structured-output provider the model can only emit an integer and the graded bands in the prompt are unreachable.

I'd fixed both Answer relevance entries and missed this one purely because I was grepping for that score name rather than auditing every schema type against its prompt.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Commit 40e08c5 addressed this comment by changing User frustration from LLM_SCHEMA_TYPE.INTEGER to LLM_SCHEMA_TYPE.DOUBLE, allowing decimal scores and graded bands.

[thiagohora]

The few-shot exemplars now demonstrate a shape the parser can't read.

The Meaning Match examples that showed the required JSON output ({"score": true, "reason": [...]}) are replaced with prose exemplars (→ TRUE — the output conveys …). Few-shot examples sitting immediately before the item to score are the strongest signal in the prompt, and they now contradict the structured-output instruction appended afterwards.

On providers without native structured output (Anthropic, Bedrock, Ollama, CUSTOM_LLM — all routed to InstructionStrategy), a judge that follows the exemplars replies → TRUE — the output matches the ground truth. toFeedbackScores then fails OBJECT_MAPPER.readTree, returns a NOT_JSON problem, and the trace gets no score — the silent no-score outcome this PR set out to remove.

If the intent was to shorten the prompt, keeping the exemplars in JSON form would preserve the demonstration without reintroducing a competing format spec.

[miguelgrc]

Fixed in 40e08c5 — the exemplars are JSON again, but in the nested schema-named shape ({"{score_name}": {"score": true, "reason": "..."}}) rather than the flat one this PR removed, so they demonstrate a parseable answer without reintroducing a competing format spec. {score_name} follows the convention the three thread templates already use, and reason is a string rather than the array that was causing reasons to be dropped.

You're right about the failure mode: on InstructionStrategy providers a judge copying → TRUE — … produces a reply that fails readTree, which is precisely the silent no-score outcome this PR exists to remove.

The same change is applied to meaning_match.mdx in both docs/ and docs-v2/, which reproduce the prompt verbatim.

🤖 Reply posted via /address-github-pr-comments

[baz-reviewer]

Commit 40e08c5 addressed this comment by replacing the prose Meaning Match exemplars with nested, schema-named JSON examples containing score and reason, preserving parseable output guidance.

[miguelgrc]

@thiagohora wrote in their review (COMMENTED):

Reviewed the parsing/escaping changes across the backend parser, Mustache renderer, and the built-in frontend templates. The direction is good — the diagnostics and the flat-score fallback are real improvements. Six inline comments below, each verified against the branch.

The tw…

Fixed in 40e08c5 — all six inline comments, plus the three lower-severity items you listed without inline comments:

• Unbounded new BigDecimal(text) — now range-checked against ValidationUtils.MIN/MAX_FEEDBACK_SCORE_VALUE, applied to numeric JSON as well as parsed text. I confirmed your severity read against a local ClickHouse: toDecimal64('1000000000', 9) raises Code 69 ARGUMENT_OUT_OF_BOUND rather than saturating, and one over the max is enough — so a single bad score would indeed have failed the whole scoreBatchOfTraces insert and lost every other score in it. The bean-validation bounds don't run here because this path builds the item directly.
• Fern pages — meaning_match.mdx updated in both docs/ and docs-v2/. I did not change answer_relevance or structure_output_compliance: those document the Python SDK metrics, whose prompts live in sdks/python/.../templates.py and are unchanged by this PR. answer_relevance_score is that metric's own field name, parsed by its own code. I edited them first and reverted once I spotted the Python placeholders ({examples_str}, {schema}) — worth flagging in case the same distinction bites elsewhere.
• Fixed Mono.delay — switched to Awaitility.await().untilAsserted(...), matching lines 473 and 643.

Also fixed one of the bot's findings in the same commit: unreadable answers now log at WARN without a stack trace, since a judge replying in prose is an expected outcome we report rather than a backend failure.

Thanks — the two you flagged as blocking were both real, and the first was a half-completed change of mine rather than a pre-existing issue.

🤖 Reply posted via /address-github-pr-comments

[github-actions]

🌿 Preview your docs: https://opik-preview-019ffa9d-cecd-7602-9a03-3a1390c2f037.docs.buildwithfern.com/docs/opik

No broken links found

Unverified links (timeout / rate-limited / server error — not failing the check)

• https://aistudio.google.com/apikey (401)
↳ on page: /docs/opik/administration/workspace-settings/ai_providers
• https://comet-ml.github.io/opik/ (timeout)
↳ on page: /docs/opik/self-host/kubernetes
• https://console.cloud.google.com/iam-admin/iam (401)
↳ on page: /docs/opik/administration/workspace-settings/ai_providers
• https://console.cloud.google.com/iam-admin/roles (401)
↳ on page: /docs/opik/administration/workspace-settings/ai_providers
• https://console.cloud.google.com/iam-admin/serviceaccounts (401)
↳ on page: /docs/opik/administration/workspace-settings/ai_providers
• https://console.mistral.ai/api-keys/ (timeout)
↳ on page: /docs/opik/integrations/mistral
• https://console.x.ai/ (403)
↳ on page: /docs/opik/integrations/xai-grok
• https://docs.predibase.com/integrations/comet (403)
↳ on page: /docs/opik/integrations/predibase
• https://drive.google.com/file/d/1b0dUc8knAncBCapo70_aTt7IwNRqOFaw/preview (timeout)
↳ on page: /docs/opik/v1/opik-university/evaluation/evaluation-ui-workflow
• https://fdr-prod-docs-files-public.s3.us-east-1.amazonaws.com/opik.docs.buildwithfern.com/48921ee0bf44fa9697977842de4f72fe8cf1f64fc1295f5103ce323cdfb884e0/img/tracing/microsoft-agent-framework-dotnet_integration.png?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Credential=AKIA6KXJSKKNFOCF7G4B%2F20260813%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20260813T101521Z&X-Amz-Expires=604800&X-Amz-Signature=d6b94efcaa784b5a2f133dd30b01d13f17a16c41081af50a22d6cd0d433c74d9&X-Amz-SignedHeaders=host&x-amz-checksum-mode=ENABLED&x-id=GetObject (403)
↳ on page: /docs/opik/v1/integrations/microsoft-agent-framework-dotnet
• https://learn.microsoft.com/en-us/semantic-kernel/concepts/enterprise-readiness/observability/telemetry-with-console?tabs=Powershell-CreateFile%2CEnvironmentFile&pivots=programming-language-python (timeout)
↳ on page: /docs/opik/v1/integrations/semantic-kernel
• https://learn.microsoft.com/en-us/semantic-kernel/overview/ (timeout)
↳ on page: /docs/opik/v1/integrations/semantic-kernel
• https://portal.azure.com/ (403)
↳ on page: /docs/opik/administration/workspace-settings/ai_providers
• https://www.together.ai/ (timeout)
↳ on page: /docs/opik/integrations/together-ai
• https://x.ai/ (403)
↳ on page: /docs/opik/integrations/xai-grok

---

📌 Results for commit c01d1fc

[github-actions]

Backend Tests - Integration Group 4

1 829 tests   1 829 ✅  9m 8s ⏱️
   20 suites      0 💤
   20 files        0 ❌

Results for commit 4bb6503.

♻️ This comment has been updated with latest results.

[CometActions]

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

[github-actions]

🔄 Test environment deployment process has started

Phase 1: Deploying base version 2.2.20-6263 (from main branch) if environment doesn't exist
Phase 2: Building new images from PR branch miguelg/OPIK-7354-fix-llm-judge-template-output-format
Phase 3: Will deploy newly built version after build completes

You can monitor the progress here.

[CometActions]

✅ 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

• URL: https://pr-7758.dev.comet.com
• Cluster: comet-ml-development
• Namespace: pr-7758
• Version: 2.2.20-7758-merge-2905
• Application logs: View in Grafana

The deployment has completed successfully and the version has been verified.

[github-actions]

Python SDK E2E Tests Results (Python 3.14)

295 tests   287 ✅  3m 34s ⏱️
  1 suites    8 💤
  1 files      0 ❌

Results for commit 77711b5.

♻️ This comment has been updated with latest results.

[CometActions]

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

[thiagohora]

Opik reviewer — follow-up pass (round 2)

Second pass on this branch, scoped to critical issues only as requested, plus a re-check of the previous round's comments.

Previous round (6 comments, 2026-08-06) — all verified fixed in the head

Comment	Status
Escape/unescape pair now asymmetric (MustacheParser)	✅ All three unescapeHtml4 calls removed from OnlineScoringEngine
Stray nested object hijacks the score name, suppresses the flat fallback	✅ declaredNames + ignoreUndeclared(); flat fallback now reached
Common textual booleans produce no score	✅ BooleanUtils.toBooleanObject + pass/passed/fail/failed map
Warning reports assertion_N instead of the user's names	✅ withUserFacingNames(...) applied before both log calls
User frustration still INTEGER	✅ Now DOUBLE
Meaning Match exemplars are prose the parser can't read	⚠️ Fixed (JSON again) — but the replacement introduced a new defect, see the llm.ts comment below

This pass

4 findings — 2 high · 2 medium. Low-severity and style findings were deliberately not posted.

Each was run through an adversarial verification gate (a verifier attempts to refute it against the repo before it posts); the Gemini one was confirmed by executing a probe against the pinned langchain4j-core 1.18.0.

Not re-raised here: the three open baz-reviewer comments from 2026-08-11 (log naming/structure, blank score names) — those are still unaddressed but are not critical.

React 👍/👎 on each comment — your feedback helps tune what it flags.

[thiagohora]

NIT

[thiagohora]

Nit: Use Lombok and move this out of this class, it's already quite big

[thiagohora]

DRY, extract it to an assertion method

[thiagohora]

Avoid hard-coded values, use dtos and podam