epam / epam/ai-dial-admin-evaluation-framework-backend

[Eval] Deployment Id validation for Response/Messages API request bodies

Open
#188 0 comments 0 reactions 1 assignee Claimed by @buhaiovos View on GitHub
enhancement P3 - Medium
Dominant language
Java
Stars
3
Forks
1
Avg merge
1d 1h
Merged PRs (30d)
30

Description

## Problem

For OpenAI chat completions, DIAL Core exposes a **deployment-specific** endpoint. `DialCoreUrlBuilder` hardcodes the suite's `deploymentRef.id` into the path:

```
/openai/deployments/{deploymentRef.id}/chat/completions
```

The request body may say anything in `model` — DIAL Core routes by URL, so the request always reaches the deployment the suite was configured for. Cost accounting, metrics, and run comparisons are therefore always attributed to the right deployment.

With the new OpenAI Responses API support (`/openai/v1/responses`, see `feat/open-api-responses`) — and with Anthropic Messages API support planned (`/anthropic/v1/messages`, TBD) — the URL is **common for all deployments**. The target deployment is selected only by the `model` property in the request body. `DialCoreUrlBuilder` passes such URLs through verbatim and `deploymentRef.id` is never used on the wire.

Consequences today:

- A suite whose `deploymentRef.id = gpt-5-mini` but whose `requestTemplate.body.model = "gpt-4o"` (or a `${{model}}` placeholder bound to a test-case field) silently calls a **different** deployment than the one recorded on the suite / run.
- Nothing in `SuiteValidationService` / `TestSuiteRequestValidator` / `TryItOutService` checks the relationship between `deploymentRef` and the body, so `isValid=true` and runs succeed with misattributed results.
- Per-run costs, deployment-level metric statistics, and run comparison are keyed off `deploymentRef`, so they would be wrong without any signal.

## Proposal

Introduce a **deployment-consistency validation** for requests routed through a shared (non-deployment-scoped) DIAL Core URL: the `model` property in the request body MUST be a **literal constant equal to `deploymentRef.id`**. Parameterisation of `model` (`${{...}}` placeholders, JSONata-computed values, bindings of any kind) is **prohibited** for these endpoints — a run is attributed to exactly one `deploymentRef`, so the deployment cannot vary per test case or be derived at resolve time.

### Scope

Applies per request in the chain (request #0 and every `additionalRequests[i]`) whenever the resolved URL is a shared-route path:

| API | Shared URL | Body property |
|-----|-----------|---------------|
| OpenAI Responses | `/openai/v1/responses` | `model` |
| Anthropic Messages (TBD) | `/anthropic/v1/messages` | `model` |

Chat completions / embeddings (`/openai/deployments/{id}/...`) and custom routes (`/v1/deployments/{id}/route/...`) are **not** affected — deployment id already lives in the URL.

### Validation rules

1. **Suite save / revalidation (soft, `isValid=false` + warning)** — in `SuiteValidationService.validateRequest`:
- `model` absent from the template body → warning `REQUIRED`, path `$.requestTemplate.body.model` (prefixed with `$.additionalRequests[i]` for chained requests): `"model is required for shared-route endpoints and must equal deploymentRef.id"`.
- `model` is a literal string ≠ `deploymentRef.id` → warning (new code, e.g. `DEPLOYMENT_MISMATCH`): `"model 'gpt-4o' does not match deploymentRef.id 'gpt-5-mini'"`.
- `model` is **parameterised** — contains a `${{placeholder}}` (whole value or embedded, any declared type), or is not a JSON string literal → warning (new code, e.g. `PARAMETERIZED_MODEL`): `"model must be a constant equal to deploymentRef.id for shared-route endpoints; placeholders are not allowed"`. This applies even if a constant binding / default would resolve it to the correct value — the rule is syntactic, not resolved-value based, so it is cheap, deterministic, and cannot be bypassed by editing bindings later.
- Any `inputBindings[*]` entry whose template variable is used (only) in `model` → same `PARAMETERIZED_MODEL` warning at `$.inputBindings[i]`.
- Body is `jsonataContent` → `model` cannot be verified statically; evaluate with defaults as already done for schema conformance and require the result to be the literal `deploymentRef.id`. Additionally, if the JSONata source references `$_request`, `data`, or any binding inside the `model` expression it SHALL be treated as parameterised. (Simplest acceptable implementation: require `model` in `jsonataContent` to be a string-literal AST node.)
2. **Try-it-out (hard, HTTP 400)** — in `TryItOutService`, after `RequestResolver.resolve(...)`: if the resolved URL is a shared-route path and resolved body `model` ≠ `deploymentRef.id` → `TryItOutValidationException`.
3. **Run time (defensive)** — in `TurnLoopExecutor` (runner core) before `deploymentTurnInvoker.invoke(...)`: check the fully resolved body `model` against `deploymentRef.id`; on mismatch the turn ends with `executionStatus = ERROR` and a JSON error envelope (`{"error":{"type":"DeploymentMismatch",...,"origin":"executor"}}`), no HTTP call is made. This guards suites saved before this validation existed and eval-cli suites that never passed through the main-app validators. Because parameterisation is rejected at save time, this guard should never fire for a valid suite.

### Design notes

- Route classification belongs next to `DialCoreUrlBuilder` (runner core) so the main app, try-it-out, and the executor share one definition of "shared-route path" — e.g. `DialCoreUrlBuilder.isSharedRoute(String resolvedUrl)` and a `Set` of shared roots mirroring `CHAT_COMPLETION_STANDARD_PATHS`.
- `model` key per API should be a constant (per bounded context constants class), not a string literal scattered across validators.
- Comparison is exact string equality with `deploymentRef.id` (DIAL deployment ids are case-sensitive).
- Placeholder detection reuses the existing `${{...}}` extraction in `TemplateVariableResolver` rather than a new regex; a `model` value that yields any template variable is parameterised.
- Do **not** attempt to "resolve then compare" for placeholders at save time — bindings and defaults are editable independently of the template, and test-case data is per row; a syntactic prohibition is the only guarantee that the value is stable across the run.
- Consider the `eval-cli` consumer of `evaluation-runner-core`: the run-time guard (rule 3) is the only one it gets, so it must live in the runner core, not in the main app.

## Honorable files

Routing / execution (`evaluation-runner-core`):
- `evaluation-runner-core/src/main/java/com/epam/aidial/evaluation/runner/service/DialCoreUrlBuilder.java` — decides deployment-scoped vs shared URL; add shared-route classification here.
- `evaluation-runner-core/src/main/java/com/epam/aidial/evaluation/runner/job/TurnLoopExecutor.java` — `urlBuilder.buildUrl(deploymentId, resolved.getUrl())` + `deploymentTurnInvoker.invoke(...)`; run-time guard goes here.
- `evaluation-runner-core/src/main/java/com/epam/aidial/evaluation/runner/service/RequestResolver.java` — produces the resolved body the run-time check runs against.
- `evaluation-runner-core/src/main/java/com/epam/aidial/evaluation/runner/service/TemplateVariableResolver.java` — `${{...}}` extraction to reuse for detecting a parameterised `model`.
- `evaluation-runner-core/src/main/java/com/epam/aidial/evaluation/runner/service/JsonataSourcePreprocessor.java` — entry point if `jsonataContent` `model` must be inspected.
- `evaluation-runner-core/src/main/java/com/epam/aidial/evaluation/runner/dto/DeploymentReferenceDto.java` — `id` is the source of truth.
- `evaluation-runner-core/src/main/java/com/epam/aidial/evaluation/runner/dto/RequestDefinitionDto.java` — chained requests carry their own `endpointRef` / `requestTemplate`.

Validation (main app):
- `src/main/java/com/epam/aidial/evaluation/service/domain/SuiteValidationService.java` — `validateDeploymentSuite` / `validateRequest`; add the soft warning per request.
- `src/main/java/com/epam/aidial/evaluation/service/domain/TestSuiteRequestValidator.java` — hard `deploymentRef` requirement lives here; touch only if a hard 400 variant is wanted.
- `src/main/java/com/epam/aidial/evaluation/service/domain/TryItOutService.java` — `validateSuitePreconditions` / `validateResolutionResult`; add the hard check on the resolved body.
- `src/main/java/com/epam/aidial/evaluation/service/domain/SuiteSnapshotBuilder.java` — snapshot must keep carrying `deploymentRef` unchanged for the run-time guard.

Specs to update:
- `openspec/specs/request-template/spec.md` — URL / body validation requirements; add "model MUST NOT be parameterised on shared-route endpoints" requirement.
- `docs/patterns/jsonata-evaluation-seam.md` — note the `model` literal constraint for `jsonataContent` bodies.
- `openspec/specs/eval-execution-engine/spec.md` — "construct the full request URL ... using `DialCoreUrlBuilder`" requirement (line ~490) + new run-time guard scenario.
- `openspec/specs/try-it-out/spec.md` — URL construction requirement (line ~384) + new 400 scenario.
- `openspec/specs/test-suites/spec.md` — validation warnings catalogue (new warning code).
- `openspec/specs/multi-request-suite/spec.md` — per-request applicability.

## Acceptance criteria

- [ ] Saving a suite with `endpointRef.relativeUrlPattern = /openai/v1/responses`, `deploymentRef.id = gpt-5-mini`, body `model = "gpt-4o"` → `isValid=false`, warning at `$.requestTemplate.body.model`.
- [ ] Same with `model = "gpt-5-mini"` → no warning.
- [ ] Same with `model = "${{model}}"` → `PARAMETERIZED_MODEL` warning **regardless** of bindings (constant `gpt-5-mini`, constant `gpt-4o`, test-case field, no binding).
- [ ] Same with an embedded placeholder (`model = "gpt-${{suffix}}"`) or non-string literal (`model = 42`, `model = null`) → `PARAMETERIZED_MODEL` / mismatch warning.
- [ ] `jsonataContent` body whose `model` is a string literal equal to `deploymentRef.id` → no warning; `model` derived from `data.*` / `$_request` / a binding → `PARAMETERIZED_MODEL` warning.
- [ ] Additional request in the chain → same rules, path `$.additionalRequests[i].requestTemplate.body.model`.
- [ ] Chat-completions suites are unaffected (no new warnings, existing functional tests green).
- [ ] Try-it-out with a mismatching resolved `model` → HTTP 400, no DIAL Core call.
- [ ] Runner: turn with mismatching resolved `model` → `ERROR` result with error envelope, no HTTP call; covered by a `TurnLoopExecutor` unit test.
- [ ] Specs above updated; `docs/patterns/multi-request-suites.md` mentions the per-request rule if applicable.

## Out of scope

- Per-test-case or per-request dynamic model selection for shared-route APIs. Explicitly prohibited by this issue; lifting it would require per-row deployment attribution in analytics, costs, and run comparison.
- Auto-filling `model` from `deploymentRef.id` when absent. Could be a follow-up UX improvement (FE or backend normalisation) but the validation must exist first.
- Actual Anthropic Messages API streaming/assembly support — tracked separately; this issue only makes the validation API-agnostic so it applies once `/anthropic/v1/messages` is routed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.