refactor(agent-service): redesign sync-execution result and error model#5927
Draft
bobbai00 wants to merge 11 commits into
Draft
refactor(agent-service): redesign sync-execution result and error model#5927bobbai00 wants to merge 11 commits into
bobbai00 wants to merge 11 commits into
Conversation
…module Foundation slice of the agent-service reorganization (no runtime behavior change): - Add src/types/api.ts (wire DTOs) and src/types/metadata.ts (operator metadata types extracted out of api/backend-api.ts); export both from the types barrel. - Add src/config/endpoints.ts exposing getServiceEndpoints(). - Move src/api/auth-api.ts -> src/auth/jwt.ts (content unchanged) and add src/auth/jwt.test.ts; update auth import paths. - Keep api/backend-api.ts transitional: it now imports/re-exports the metadata types from ../types/metadata and retains getBackendConfig/fetchOperatorMetadata, which the follow-up clients PR relocates. The types/agent.ts and types/workflow.ts reshaping is deferred to the PR that also updates server.ts/workflow-state.ts, since those renames are coupled.
Split types/api.ts into types/wire.ts (backend wire DTOs) and types/ws.ts
(this service's own WebSocket frames), and consume them at the real call
sites instead of the duplicate definitions that previously lived inline:
- server.ts imports WsMessage/WsOutgoingMessage/OperatorResultSummaryWs from
types/ws (local copies removed).
- api/workflow-api.ts imports Workflow/WorkflowPersistRequest from types/wire.
- api/compile-api.ts, agent/util/context-utils.ts, and agent/texera-agent.ts
import WorkflowFatalError/WorkflowCompilationResponse from types/wire.
Correct WorkflowFatalError to match the backend proto (workflowruntimestate.proto):
`type` is the FatalErrorType enum name (string), with details/operatorId/
workerId/timestamp optional, replacing the inaccurate `type: { name }` /
all-required shape. Type-level only; consumers read only `.message`, so there
is no runtime behavior change.
tsc --noEmit, prettier --check, and bun test (101 pass / 0 fail) all green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Encode test JWT segments as base64url (not base64) to match real tokens, including a case whose payload contains `-`/`_` to pin url-safe decoding. - Add extractBearerToken coverage: valid bearer, case-insensitive scheme, non-Bearer scheme, missing token, absent header. Addresses the Copilot review comments on src/auth/jwt.test.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure file rename plus import-path updates; no type or behavior changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hema from types/metadata workflow-system-metadata.ts defined its own copies of these two interfaces, identical to the canonical ones in types/metadata.ts. Import the canonical types and delete the duplicates so the centralized definitions are actually used. Type-level only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…initions Remove all `any` from src/types/*.ts: - opaque/object JSON blobs -> unknown / Record<string, unknown> (jsonSchema, operatorProperties, result rows, sampleRecords, ReActStep tool input/output, physicalPlan, OperatorSchemaInfo/CompactOperatorSchema). - WsOutgoingMessage.workflowContent -> WorkflowContent (it is assigned from WorkflowState.getWorkflowContent()). Type-level only; tsc, prettier, and tests pass with no consumer changes needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o *.spec.ts - Rename all *.test.ts to *.spec.ts (bun discovers .spec by default; no config change). - Add workflow-system-metadata.spec.ts: schema compaction ($ref inlining, key filtering), getAllSchemasAsJson, Ajv property validation, and the formatting helpers. - Add workflow-api.spec.ts and compile-api.spec.ts: persist/retrieve/compile over a mocked fetch, exercising the proto-accurate WorkflowFatalError shape. 126 pass / 0 fail; tsc and prettier green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ReActStep used `unknown` for fields whose shapes we actually know: - inputMessages -> ModelMessage[] (the prepared AI SDK messages) - toolCalls[].input -> Record<string, unknown> (tool args are JSON objects) - toolResults[].output -> string (every tool returns via createToolResult/createErrorResult) The AI SDK types tc.input / tr.output as `unknown` for dynamically registered tools, so narrow at the single construction boundary in texera-agent.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ted unions Replace the flat src/types/ws.ts with a types/ws/ directory: - client.ts: WsClientRequest = WsClientRequestPrompt | WsClientRequestStopCommand (each extending a base; orthogonal fields instead of one all-optional interface) - server.ts: WsServerMessage union (init/step/state/complete/error/headChange), each declaring only the fields it sends; OperatorResultSummaryWs moves here - index.ts: barrel so types/index.ts's `export * from "./ws"` resolves to the dir The client->server wire discriminator changes from "message"/"stop" to "prompt"/"command" (stop becomes commandType:"stop"); server->client `type` values are unchanged. server.ts parses WsClientRequest and switches on the new shapes, and the frontend agent.service.ts WS sends are updated in lockstep so the protocol stays consistent end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ndation PR The execution-result redesign (OperatorExecutionSummary etc.) lives in a follow-up PR; revert the lone any->unknown tweak so this PR leaves types/execution.ts byte-identical to main.
Restructure the per-operator summary the sync-execution backend returns and the
agent-service/frontend consume, for a leaner and consistent wire contract:
- Replace flat OperatorInfo with OperatorExecutionSummary: state, errorMessages,
resultSummary?, consoleLogsSummary? (orthogonal sub-summaries; no shape stats).
- Rename SyncExecutionResult -> WorkflowExecutionSummary; drop compilationErrors
(folded into errors). errors and per-op errorMessages are non-optional (empty
means none).
- OperatorResultSummary.sampleTuples is now List[SampleRow] ({rowIndex, tuple})
instead of a JSON array with an embedded __row_index__. Drop the table-shape
types (TableShape/InputPortTableShape): the agent derives input-port shapes
from the DAG + each upstream's output shape; output shape comes from the result
summary.
- Reuse the engine's WorkflowFatalError for per-operator errors (the same type
the compiling service returns for compilation errors), replacing the bespoke
OperatorError so compile and execution errors share one wire shape.
- Collapse console messages onto one type; derive warnings from WARNING-titled
messages rather than a separate field.
- Replace OperatorResultSummaryWs: the WS operatorResults payload now carries the
canonical OperatorExecutionSummary; the frontend maps it to its flat display
type (re-flattening sampleTuples to keep the display components unchanged).
Touches the Scala producer (SyncExecutionResource), the agent-service consumers
(result-formatting, workflow-execution-tools, workflow-result-state, server) and
the frontend WS mapping. Representation/type-level change; behavior preserved,
except input-port shape lines are now derived rather than explicitly rendered.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qFkyrpTd5PrkNBPcBeo4Q
github-actions
Bot
added
engine
refactor
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5927 +/- ##
============================================
+ Coverage 54.60% 55.19% +0.59%
Complexity 2927 2927
============================================
Files 1109 1109
Lines 42828 42630 -198
Branches 4608 4604 -4
============================================
+ Hits 23385 23529 +144
+ Misses 18081 17738 -343
- Partials 1362 1363 +1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
✅ No material benchmark regressions detected🟢 0 better · 🔴 0 worse · ⚪ 15 noise (<±5%) · 0 without baseline
Baseline detailsLatest main
Raw CSVconfig_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,455.19,200,128000,439,0.268,21712.58,31124.48,31124.48
1,100,10,64,20,2072.22,2000,1280000,965,0.589,104817.72,118184.64,118184.64
2,1000,10,64,20,17957.45,20000,12800000,1114,0.680,899322.80,950203.24,950203.24 |
This was referenced Jun 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Stacked on #5751 (foundation). This is the second of the split: it carries the sync-execution result/error model redesign (no overlap with the foundation PR —
types/execution.tsis untouched there).OperatorInfowithOperatorExecutionSummary(orthogonal sub-summaries:state,errorMessages,resultSummary?,consoleLogsSummary?); renameSyncExecutionResult→WorkflowExecutionSummary.resultSummary.sampleTuplesis nowSampleRow[]({ rowIndex, tuple }) instead of JSON rows with an embedded__row_index__; drop the table-shape types (the agent derives input-port shapes from the DAG).WorkflowFatalErrorfor per-operator errors (the same type the compiling service returns), replacing the bespokeOperatorErrorso compile and execution errors share one wire shape.errorMessages/errorsare non-optional (empty = none); dropcompilationErrors; collapse the console-message types and derive warnings fromWARNING:-titled messages.operatorResultspayload carries the canonicalOperatorExecutionSummary; the frontend maps it to its flat display type.Touches the Scala producer (
SyncExecutionResource), the agent-service consumers (result-formatting,workflow-execution-tools,workflow-result-state,server), and the frontend WS mapping. Representation/type-level; behavior preserved (input-port shape lines are now derived rather than explicitly rendered).Any related issues, documentation, discussions?
Closes #5750
Part of #5747.
How was this PR tested?
bunx tsc --noEmit,bun test(121 pass / 0 fail),prettier --checkinagent-service;sbt WorkflowExecutionService/compilefor amber./operator-resultsreturned the new shape —resultSummary.sampleTuples: [{ rowIndex, tuple }],errorMessages: []— and the agent rendered the rows correctly.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8 (1M context)