Skip to content

refactor(sync-execution): migrate agent result contract#6314

Draft
bobbai00 wants to merge 1 commit into
apache:mainfrom
bobbai00:refactor/sync-execution-agent-contract
Draft

refactor(sync-execution): migrate agent result contract#6314
bobbai00 wants to merge 1 commit into
apache:mainfrom
bobbai00:refactor/sync-execution-agent-contract

Conversation

@bobbai00

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

This is PR 1 of the split from #6009. It migrates the sync-execution backend and agent-service internals to the new execution summary contract while keeping the current frontend response shape compatible.

Core changes:

  • Replace the sync-execution result model with WorkflowExecutionSummary, OperatorExecutionSummary, and OperatorResultSummary.
  • Represent sampled rows as [rowIndex, Tuple] pairs instead of embedding row metadata into JSON result records.
  • Reuse the shared WorkflowFatalError shape for execution and compilation-style failures.
  • Update agent-service execution formatting, result cache, server paths, and tests to consume the new model.
  • Keep GET /agents/:id/operator-results legacy-compatible through a temporary adapter, so the existing frontend continues to work until the follow-up frontend PR lands.

Main review files:

  • amber/src/main/scala/org/apache/texera/web/resource/SyncExecutionResource.scala
  • agent-service/src/types/execution.ts
  • agent-service/src/server.ts

Any related issues, documentation, discussions?

Part of #5750.
Part of #5747.
Split from #6009.

A follow-up PR will migrate the frontend to consume the new summary shape directly and remove the temporary compatibility adapter.

How was this PR tested?

  • cd agent-service && bun test src/server.spec.ts src/agent/tools/result-formatting.spec.ts src/agent/tools/tools-utility.spec.ts src/agent/tools/workflow-execution-tools.spec.ts src/agent/workflow-result-state.spec.ts
  • cd agent-service && bun run format:check
  • cd agent-service && bun run typecheck
  • sbt scalafmtCheckAll

Attempted but did not complete locally:

  • sbt "WorkflowExecutionService / testOnly org.apache.texera.web.resource.SyncExecutionResourceSpec"

The command failed before reaching the spec because local JOOQ generation could not authenticate to Postgres, then existing generated Java deprecation warnings hit -Werror during DAO / Compile.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Codex

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Ma77Ball, @Yicong-Huang
    You can notify them by mentioning @Ma77Ball, @Yicong-Huang in a comment.

@codecov-commenter

codecov-commenter commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.35484% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.58%. Comparing base (1677e43) to head (ffaf65b).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
...he/texera/web/resource/SyncExecutionResource.scala 94.35% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6314      +/-   ##
============================================
+ Coverage     60.68%   61.58%   +0.90%     
- Complexity     3367     3444      +77     
============================================
  Files          1134     1134              
  Lines         44167    43950     -217     
  Branches       4827     4834       +7     
============================================
+ Hits          26802    27066     +264     
+ Misses        15907    15380     -527     
- Partials       1458     1504      +46     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 1677e43
agent-service 44.23% <ø> (-0.37%) ⬇️ Carriedforward from 1677e43
amber 68.34% <94.35%> (+2.20%) ⬆️
computing-unit-managing-service 0.00% <ø> (ø) Carriedforward from 1677e43
config-service 52.30% <ø> (ø) Carriedforward from 1677e43
file-service 63.97% <ø> (ø) Carriedforward from 1677e43
frontend 52.50% <ø> (ø) Carriedforward from 1677e43
notebook-migration-service 78.57% <ø> (ø) Carriedforward from 1677e43
pyamber 91.18% <ø> (ø) Carriedforward from 1677e43
workflow-compiling-service 55.14% <ø> (ø) Carriedforward from 1677e43

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

✅ No material benchmark regressions detected

🟢 4 better · 🔴 0 worse · ⚪ 11 noise (<±5%) · 0 without baseline

Compared against main 1b42558 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🟢 bs=10 sw=10 sl=64 429 0.262 22,615/31,606/31,606 us 🟢 -8.8% / 🔴 +109.1%
🟢 bs=100 sw=10 sl=64 938 0.573 104,367/126,451/126,451 us 🟢 -10.7% / 🔴 +18.1%
bs=1000 sw=10 sl=64 1,113 0.68 897,113/941,193/941,193 us ⚪ within ±5% / 🟢 -11.0%
Baseline details

Latest main 1b42558 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 429 tuples/sec 429 tuples/sec 779.42 tuples/sec 0.0% -45.0%
bs=10 sw=10 sl=64 MB/s 0.262 MB/s 0.262 MB/s 0.476 MB/s 0.0% -44.9%
bs=10 sw=10 sl=64 p50 22,615 us 23,590 us 12,550 us -4.1% +80.2%
bs=10 sw=10 sl=64 p95 31,606 us 34,647 us 15,115 us -8.8% +109.1%
bs=10 sw=10 sl=64 p99 31,606 us 34,647 us 18,652 us -8.8% +69.5%
bs=100 sw=10 sl=64 throughput 938 tuples/sec 928 tuples/sec 1,000 tuples/sec +1.1% -6.2%
bs=100 sw=10 sl=64 MB/s 0.573 MB/s 0.566 MB/s 0.61 MB/s +1.2% -6.1%
bs=100 sw=10 sl=64 p50 104,367 us 102,539 us 99,872 us +1.8% +4.5%
bs=100 sw=10 sl=64 p95 126,451 us 141,556 us 107,073 us -10.7% +18.1%
bs=100 sw=10 sl=64 p99 126,451 us 141,556 us 114,377 us -10.7% +10.6%
bs=1000 sw=10 sl=64 throughput 1,113 tuples/sec 1,121 tuples/sec 1,020 tuples/sec -0.7% +9.1%
bs=1000 sw=10 sl=64 MB/s 0.68 MB/s 0.684 MB/s 0.622 MB/s -0.6% +9.2%
bs=1000 sw=10 sl=64 p50 897,113 us 896,636 us 987,499 us +0.1% -9.2%
bs=1000 sw=10 sl=64 p95 941,193 us 928,974 us 1,028,784 us +1.3% -8.5%
bs=1000 sw=10 sl=64 p99 941,193 us 928,974 us 1,057,136 us +1.3% -11.0%
Raw CSV
config_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,466.09,200,128000,429,0.262,22615.32,31605.98,31605.98
1,100,10,64,20,2131.41,2000,1280000,938,0.573,104367.11,126450.87,126450.87
2,1000,10,64,20,17964.34,20000,12800000,1113,0.680,897112.58,941193.19,941193.19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants