发布

RSS 订阅源

2.0.17-5168 d254b9355e

比较
[OPIK-6048] [BE] feat: add dedicated assertion-results batch endpoints (#6391)

frostbyte_neo 发布于 2026-04-28 11:43:31 +00:00
- [OPIK-6048] [BE] feat: add dedicated assertion-results batch endpoints
Introduce PUT /v1/private/traces/assertion-results and PUT /v1/private/spans/assertion-results
that call AssertionResultService directly with an assertion-native DTO (AssertionResultBatch /
AssertionResultBatchItem carrying id, projectName, name, passed, reason, source) — no
FeedbackScoreItem coupling. AssertionResultService gains saveBatchOfTraces / saveBatchOfSpans,
resolves project names, persists via a new AssertionResultDAO.saveBatch method, and emits both
the new AssertionResultsCreated event and the existing FeedbackScoresCreated event for
ExperimentAggregateEventListener backward compatibility.

The legacy FeedbackScoreService.saveScoreBatch path — including the category_name == "suite_assertion"
routing — is intentionally kept so existing SDK and frontend clients keep working. Migration of
SDKs / frontend and removal of the category_name fallback are deferred to OPIK-5192 subtasks 2 & 3.

Implements OPIK-6048.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- refactor(assertion-results): align new DTOs/service with SKILL.md
- Add @NonNull on required AssertionResultBatchItem components and AssertionResultBatch.assertionResults; reject null entries with List<@NotNull @Valid ...>
- Drop @NonNull from new AssertionResultDAO.saveBatch and AssertionResultService.saveBatchOfTraces/saveBatchOfSpans interface parameters (SKILL.md forbids validation annotations on interface contracts)
- Wrap log interpolation values in single quotes in the new /assertion-results endpoints
Legacy insertBatch(List<? extends FeedbackScoreItem>) deliberately left untouched — it is only kept for the backward-compat path through FeedbackScoreService.saveScoreBatch and will be removed with OPIK-5192 subtask 3.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- refactor(assertion-results): switch new batch endpoints from @PUT to @POST
/assertion-results is a collection URI, not an individual resource, so POST is the
semantically correct verb for a batch create. Storage-level idempotency (ReplicatedReplacingMergeTree
- FROM FINAL) is an implementation detail; it should not leak into the HTTP contract.
Updates both resource methods and the matching resource-client test helpers. The legacy
/feedback-scores endpoints remain @PUT — they are frozen for backward compat and will be
retired with OPIK-5192 subtask 3.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- refactor(assertion-results): dedicated resource, PUT verb, enum payload, single event
Address PR review (andrescrz, CHANGES_REQUESTED):
- Extract a dedicated AssertionResultsResource at /v1/private/assertion-results
  with a single PUT endpoint that takes entity_type in the body. Removes
  /assertion-results from TracesResource and SpansResource. Resolves
  comments on resource cohesion / single-endpoint shape (A1, A2, A3, A4).
- Switch verb back to PUT. assertion_results is a ReplicatedReplacingMergeTree
  with last_updated_at as version and FROM FINAL reads, so posting the same
  batch twice is idempotent at the contract level — matches the legacy
  /feedback-scores sibling. Resolves B1.
- AssertionResultBatchItem.passed: Boolean -> AssertionStatus enum. DAO binds
  the enum value directly, no value>=1 threshold. Forward-compatible for new
  states. Resolves D1.
- Drop Lombok @NonNull from request-body DTOs and DAO interface params; rely
  on Jakarta validation (@NotNull, @NotBlank, @Size, @Valid). Resolves
  E1, E2, E3, E4.
- AssertionResultService: collapse saveBatchOfTraces / saveBatchOfSpans into
  a single saveBatch(EntityType, List<...>) method on the interface. Validates
  entity type is TRACE or SPAN (rejects THREAD with 400).
- Replace .doOnSuccess with .then(Mono.fromRunnable(...)) so events fire on
  successful completion only. Drop the dual FeedbackScoresCreated emission
  and wire ExperimentAggregateEventListener to subscribe to the new
  AssertionResultsCreated event directly. Resolves G1, G2.
- Method/operationId: storeAssertionsBatch (vs the verbose
  saveBatchOfSpanAssertionResults). Resolves C1.
- Tests: new AssertionResultsResourceClient driving the dedicated endpoint;
  expanded AssertionResultsResourceTest covers entity_type=THREAD rejection,
  missing entity_type, missing passed, invalid passed enum value, multi-project
  resolution. Resolves H1.
- SKILL.md: clarify the @NonNull-vs-@NotNull rule under "Records and DTOs"
  — Lombok @NonNull for internal records, Jakarta validators for request-body
  DTOs validated via @Valid, never stack both on the same field. Codifies the
  convention so future contributors don't repeat the mistake this PR went
  through.
projectName/projectId mutual exclusion (F1) intentionally not added — matches
existing FeedbackScoreBatchItem convention; no mutex enforced anywhere in the
codebase today, and adding it only here would diverge.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- refactor(assertion-results): rename status/entityId, tighten validation, persistence-asserting tests
Address PR review (andrescrz, round 3):
- Rename passed -> status and id -> entityId on AssertionResultBatchItem.
  Both fields had misleading names: passed is an AssertionStatus (not a boolean),
  and id reads like an assertion result id when it's the trace/span entity id.
  JSON field, DAO bind, service references, and tests all updated.
- AssertionResultBatch now declares per-element validation:
  List<@NotNull @Valid AssertionResultBatchItem>.
- AssertionResultDAOImpl.saveBatch and AssertionResultServiceImpl.saveBatch get
  @NonNull on impl params (interface stays bare per SKILL.md).
- Empty assertionResults list at the service now throws BadRequestException
  rather than returning Mono.empty() silently. The Jakarta @Size(min=1) catches
  it first at the resource layer; this guards future internal callers.
- ExperimentAggregateEventListener.onAssertionResultsCreated logs workspaceId,
  entityType, and entityIds size on receipt and on aggregation failure.
- AssertionResultsResourceClient uses Lombok @RequiredArgsConstructor.
Tests rewrite per reviewer feedback:
- Parameterise the trace/span happy path via @EnumSource over EntityType.
- Strengthen success-case assertions: query assertion_results FINAL via injected
  TransactionTemplateAsync and verify rows are persisted with the right
  entity_type / passed / reason / source / project_id values.
- Cover multi-project resolution with persistence assertions on both projects.
- Drop low-value tests that exercised only Jakarta/Dropwizard validation paths
  (missingEntityType, missingName, missingPassed, invalidPassedEnum, emptyBatch).
  Keep code-path tests: THREAD entity-type rejection, non-v7 UUID rejection,
  multi-project resolution.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- refactor(assertion-results): align DAO bind key with renamed status field
Rename the SQL parameter placeholder and bind key from passed to status so the
DAO reads consistently end-to-end with the renamed AssertionResultBatchItem.status
field. The assertion_results.passed column name is unchanged — only the placeholder
in the VALUES clause and the bind keys move.

Both bindParameters (legacy FeedbackScoreItem path, kept for backward compat) and
bindAssertionResultParameters (new path) bind through the same SQL template, so both
must use the new placeholder name.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- test(assertion-results): verify persistence through experiment-items API, drop direct CH query
Address PR review (andrescrz, round 4):
- Drop the TransactionTemplateAsync injection and the AssertionResultRow private
  record that were used to query assertion_results FINAL directly. Per reviewer:
  "Tests should be done through the APIs, not by directly querying the DB."
- Parameterise happy path keeps both EntityType.TRACE and EntityType.SPAN, but
  now verifies only the 204 status code. Span-level assertion reads are not
  surfaced by any current public API (every reader filters entity_type='trace'),
  so the SPAN write path is exercised as a status-code smoke test only.
- Add a dedicated TRACE-only persistence test
  (traceAssertionsArePersistedAndRetrievableViaExperimentItems) that sets up a
  TEST_SUITE experiment + dataset item linked to the trace, posts assertions via
  the new endpoint, and retrieves them through experimentResourceClient.getExperimentItems,
  asserting the full ExperimentItem.assertionResults collection with
  containsExactlyInAnyOrder over full AssertionResult records (no field-by-field).
- Rewire multiProjectBatchResolvesIndependently to set up a separate experiment
  per project and verify each via the experiment-items API.
- nonV7EntityIdIsRejected now asserts the exact 400 status (InvalidUUIDVersionException
  extends BadRequestException) instead of the loose isBetween(400, 499).
- Helper setUpExperimentItemForTrace encapsulates the dataset/experiment/
  experiment-item scaffolding shared by the TRACE persistence tests.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- refactor(assertion-results): tighten test surface and clarify validation flow
Address PR review (round 5):
- Drop the redundant storeAssertionsBatch_returns204 parameterised happy path.
  The dedicated traceAssertionsArePersistedAndRetrievableViaExperimentItems test
  already covers the TRACE write path through the public read API. SPAN persistence
  verification waits on a span-level read API (tracked in OPIK-6255).
- Convert threadEntityTypeIsRejected into unsupportedEntityTypeIsRejected,
  parameterised over every EntityType value except TRACE and SPAN. Future
  EntityType additions will be covered automatically and asserted to return 400.
- Remove the now-orphaned createEntity helper, spanResourceClient, and
  Span import that only existed for the dropped happy path.
- Extract the IdGenerator.validateVersion call from inside the Stream.map
  onto a dedicated forEach pre-pass before grouping, so the grouping loop
  focuses on project-name normalisation only. Behaviour unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

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

0 发布 8,340 Git 标签

[OPIK-6048] [BE] feat: add dedicated assertion-results batch endpoints (#6391)