[OPIK-7396] [BE] fix: tolerate >1 emitted row in TraceDAO.findById (get-by-id 500) by thiagohora · Pull Request #7532 · comet-ml/opik

thiagohora · 2026-07-20T13:37:36Z

Details

GET /v1/private/traces/{id} intermittently returned 500 with IndexOutOfBoundsException: "Source emitted more than one item", and — once the reducer was made lenient — could return a non-deterministic experiment field. Root cause: in SELECT_BY_IDS, the outer LEFT JOIN experiments_agg eaag ON eaag.trace_id = t.id had no per-trace dedup, so a trace that belongs to N experiments fanned out into N rows (differing experiment_id/name/dataset_id).

Fix at the source: experiments_agg now collapses to a single canonical row per trace_id — the most recent experiment (ORDER BY trace_id, experiment_id DESC + LIMIT 1 BY trace_id, ids are UUIDv7 / time-ordered). The trace-by-id join therefore returns exactly one row. This also stops batch getByIds returning duplicate rows for multi-experiment traces. Trace.experiment is a single field, so collapsing loses nothing at the API level.
Defense in depth: findById reduces via collectList() + first and logs a WARN (trace id + workspace) if a fan-out ever reappears, so a future regression degrades to a log + one row instead of a 500.

Change checklist

User facing
Documentation update

Issues

Resolves OPIK-7396

AI-WATERMARK

AI-WATERMARK: yes

Tools: Claude Code
Model(s): Claude Opus 4.8
Scope: Production diagnosis, the query-level collapse in SELECT_BY_IDS, the defensive reducer, and the unit test.
Human verification: Author reviewed the diff, confirmed the query change against the fan-out analysis, and confirmed the test run below.

Testing

mvn -o test-compile + spotless:apply clean.
Unit test TraceDAOImplTest on the reducer (no DB): no rows → empty; single → that; >1 → first, no IndexOutOfBoundsException. Tests run: 3, Failures: 0.
The query-level collapse is DB-backed; a regression test (a trace in ≥2 experiments → GET /{id} returns 200 with a deterministic experiment) belongs in the traces integration suite (CI). Flagged on the ticket.

Documentation

No documentation changes — internal reliability/correctness fix; no API shape change (Trace.experiment remains a single value).

GET /v1/private/traces/{id} 500s with IndexOutOfBoundsException ("Source emitted more than one item") when the assembled get-by-id query fans out to >1 row for the id. findById reduced with singleOrEmpty(), which throws on >1. Use next() so the first row is returned and the empty case still maps to 404. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-07-20T13:38:57Z

⏱️ pre-commit per-hook timing

Hook	Description	Result	Duration
☕ spotless — java backend	Format Java code	✅	4.28s
Total (1 ran)			4.28s

⏭️ 40 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 — 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	⏭️

Replace next() with collectList()+first so we can emit a WARN (trace id + row count) when the get-by-id query fans out to more than one row, keeping the underlying duplication observable instead of silently dropping the extra rows. Behaviour is unchanged: empty -> 404, otherwise the first row is returned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Read WORKSPACE_ID from the reactive context (as findByIds does) so the fan-out WARN carries both the trace id and the workspace, making the duplication easier to attribute. No behavioural change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Extract the get-by-id row reduction into a package-private static (firstOrLogFanOut) and unit-test it: empty -> empty, single -> that row, and >1 -> first row without throwing IndexOutOfBoundsException. Pure unit test, no DB harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…er trace The get-by-id 500 (and the reducer's non-deterministic experiment pick) came from SELECT_BY_IDS fanning a trace into one row per experiment: the outer LEFT JOIN experiments_agg had no per-trace dedup. Collapse experiments_agg to a single canonical row per trace_id (most recent experiment by UUIDv7-ordered id) so the trace-by-id join returns exactly one row. Also fixes batch getByIds returning duplicate rows for multi-experiment traces. The findById reducer stays as a defensive detector (logs + returns first if a fan-out ever reappears). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

JetoPistola · 2026-07-20T21:01:35Z

❓ question | Logical Bugs — adjacent query (out of scope for this PR)

Posting as a general comment since the referenced lines aren't in this PR's diff.

The sibling list query SELECT_BY_PROJECT_ID looks like it has the same experiments_agg fan-out this PR just fixed in SELECT_BY_IDS:

experiments_agg AS (SELECT DISTINCT ...) at TraceDAO.java:1260 has no LIMIT 1 BY trace_id collapse (unlike the fixed one, which now does at line 715).
It's joined in the final SELECT via LEFT JOIN experiments_agg eaag ON eaag.trace_id = t.id (TraceDAO.java:1459), and that final SELECT ends with ORDER BY ... + SETTINGS ... — no outer LIMIT 1 BY id. The LIMIT 1 BY id at line 1428 is inside the page_wide CTE, which is then re-joined to experiments_agg afterward.

So a trace belonging to ≥2 experiments would seem to fan the list result into duplicate trace rows there too — the same class of bug, minus the 500 (the list reducer isn't strict-single).

This PR is correctly scoped to the get-by-id 500, so not a blocker here. Is the list-query fan-out already known/tracked, or worth a follow-up ticket? (Matches the recurring CTE fan-out pattern.)

🤖 Review posted via /review-github-pr

thiagohora · 2026-07-21T07:46:34Z

Good catch, and correct. Verified: SELECT_BY_PROJECT_IDs experiments_agg has no LIMIT 1 BY trace_id, and its final SELECT ends ORDER BY … SETTINGS … with no outer LIMIT 1 BY id (the LIMIT 1 BY id is inside the page_wide CTE, which is then re-joined to experiments_agg). So a trace in ≥2 experiments fans the list into duplicate rows — same class as this PR, minus the 500, and only when the experiment join is active (sort_has_experiment || !exclude_experiment).

Agreed it is out of scope here (this PR is the get-by-id 500). It was not yet tracked — filed a follow-up: OPIK-7416, with the same collapse (LIMIT 1 BY trace_id, most-recent experiment) as the fix. Thanks!

andrescrz

Just minor recommendations, otherwise LGTM.

andrescrz · 2026-07-21T10:35:47Z

    public Mono<Trace> findById(@NonNull UUID id, @NonNull Connection connection) {
        return findByIds(List.of(id), connection)
-                .singleOrEmpty();
+                .collectList()


Minor: for checking size > 1 and logging and checking the first one, there might be an easy way to avoid collecting all results (waits and accumulates in memory). Ok to keep this if the alternative is too complicated and not a clean implementation. Results should be small anyway.

Keeping collectList() here. Now that SELECT_BY_IDS collapses experiments_agg to one row per trace_id, this buffers at most one row (empty on miss), so there is nothing to accumulate in practice — the reducer stays only as a defensive fan-out detector. Agree the alternative (bounded reactive reduce) is not worth the added complexity here.

@VisibleForTesting

- Annotate firstOrLogFanOut with @VisibleForTesting (it exists for the unit test). - Move the interpolated values to the end of the fan-out WARN (grep/service convention). Kept collectList(): SELECT_BY_IDS now collapses experiments_agg to one row per id, so the reducer buffers at most one row; it remains only as a defensive fan-out detector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions Bot assigned thiagohora Jul 20, 2026

github-actions Bot added java Pull requests that update Java code Backend 🔵 size/XS labels Jul 20, 2026

baz-reviewer Bot added the baz: pending label Jul 20, 2026

thiagohora and others added 3 commits July 20, 2026 15:42

github-actions Bot added tests Including test files, or tests related like configuration. 🟢 size/S and removed 🔵 size/XS labels Jul 20, 2026

thiagohora marked this pull request as ready for review July 20, 2026 13:53

thiagohora requested a review from a team as a code owner July 20, 2026 13:53

baz-reviewer Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread apps/opik-backend/src/main/java/com/comet/opik/domain/TraceDAO.java

Comment thread apps/opik-backend/src/main/java/com/comet/opik/domain/TraceDAO.java

Comment thread apps/opik-backend/src/test/java/com/comet/opik/domain/TraceDAOImplTest.java

baz-reviewer Bot approved these changes Jul 20, 2026

View reviewed changes

andrescrz previously approved these changes Jul 21, 2026

View reviewed changes

thiagohora dismissed andrescrz’s stale review via df90754 July 21, 2026 14:16

thiagohora requested a review from andrescrz July 21, 2026 14:18

andrescrz approved these changes Jul 21, 2026

View reviewed changes

thiagohora merged commit f570882 into main Jul 21, 2026
71 checks passed

thiagohora deleted the thiaghora/OPIK-7396-traces-getbyid-next branch July 21, 2026 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[OPIK-7396] [BE] fix: tolerate >1 emitted row in TraceDAO.findById (get-by-id 500)#7532

[OPIK-7396] [BE] fix: tolerate >1 emitted row in TraceDAO.findById (get-by-id 500)#7532
thiagohora merged 6 commits into
mainfrom
thiaghora/OPIK-7396-traces-getbyid-next

thiagohora commented Jul 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JetoPistola commented Jul 20, 2026

Uh oh!

thiagohora commented Jul 21, 2026

Uh oh!

andrescrz left a comment

Uh oh!

andrescrz Jul 21, 2026

Uh oh!

thiagohora Jul 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

thiagohora commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Change checklist

Issues

AI-WATERMARK

Testing

Documentation

Uh oh!

github-actions Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏱️ pre-commit per-hook timing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JetoPistola commented Jul 20, 2026

Uh oh!

thiagohora commented Jul 21, 2026

Uh oh!

andrescrz left a comment

Choose a reason for hiding this comment

Uh oh!

andrescrz Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

thiagohora Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thiagohora commented Jul 20, 2026 •

edited

Loading

github-actions Bot commented Jul 20, 2026 •

edited

Loading