Skip to content

refactor(workflow-operator): share the chart output contract - #9204

Open
aglinxinyuan wants to merge 1 commit into
apache:mainfrom
aglinxinyuan:refactor/visualization-output-trait
Open

aglinxinyuan wants to merge 1 commit into
apache:mainfrom
aglinxinyuan:refactor/visualization-output-trait

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

The 48 Python visualization descriptors each restated the same output contract: a getOutputSchemas override returning one html-content STRING column on the single output port, and producesDataFrame = false so the exported script does not print the chart as a leaf DataFrame. A new trait, PythonVisualizationOpDesc, now declares both, and the 48 descriptors extend it. The 41 Plotly charts keep with PlotlyStandaloneCode.

Surface Pinned
output schema one html-content STRING column on the single output port, independent of the input schemas
exported script not printed as a leaf DataFrame
operator metadata JSON schema and operator info unchanged for all 166 operators

The trait has no fields, so no JSON schema changes. VisualizationOutputSchemaSpec still sweeps every registered chart and remains the guard; only its doc comment changed, to name the trait.

Any related issues, documentation, discussions?

Closes #9203.

How was this PR tested?

  • sbt "WorkflowOperator/test": each of its 2,770 tests (296 suites) has the same result before and after this change, compared test by test from the JUnit XML reports. Three tests fail on main and on this branch alike on the Windows machine used (a locked temp file in FileScanSourceOpDescSpec, a mount path in PythonUdfUiParameterSupportSpec, an exported-script comparison in UrlVizOpDescSpec); none touches the changed code.
  • sbt "WorkflowCompilingService/testOnly org.apache.texera.amber.translator.*": 41 tests pass before and after.
  • The JSON schema and operator info of all 166 registered operators, dumped before and after, are identical.
  • sbt "WorkflowOperator/scalafmtCheck" "WorkflowOperator/Test/scalafmtCheck" "WorkflowOperator/scalafix --check" "WorkflowOperator/Test/scalafix --check"

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

Generated-by: Claude Code (Claude Opus 5.5)

The 48 Python visualization descriptors each overrode getOutputSchemas to
return one html-content STRING column and set producesDataFrame to false.
PythonVisualizationOpDesc now declares both, and the descriptors extend
it. Schemas, generated Python and operator metadata are unchanged.
Copilot AI balanced review requested due to automatic review settings October 28, 2026 08:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@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: @kz930, @eugenegujing
    You can notify them by mentioning @kz930, @eugenegujing in a comment.

@codecov-commenter

codecov-commenter commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.03922% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.04%. Comparing base (92e86d4) to head (712c9b1).

Files with missing lines Patch % Lines
...ator/visualization/PythonVisualizationOpDesc.scala 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9204      +/-   ##
============================================
+ Coverage     91.98%   92.04%   +0.06%     
+ Complexity     5083     5039      -44     
============================================
  Files          1285     1286       +1     
  Lines         56094    55910     -184     
  Branches       7137     7137              
============================================
- Hits          51599    51465     -134     
+ Misses         2742     2694      -48     
+ Partials       1753     1751       -2     
Flag Coverage Δ *Carryforward flag
access-control-service 77.44% <ø> (ø)
agent-service 99.16% <ø> (ø) Carriedforward from 92e86d4
amber 86.56% <98.03%> (+0.12%) ⬆️
computing-unit-managing-service 64.02% <ø> (ø)
config-service 87.61% <ø> (ø)
file-service 82.29% <ø> (ø)
frontend 96.47% <ø> (ø) Carriedforward from 92e86d4
notebook-migration-service 83.77% <ø> (ø)
pyamber 98.54% <ø> (ø) Carriedforward from 92e86d4
workflow-compiling-service 74.25% <ø> (ø)

*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

⚠️ Benchmark changes need a look

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

Compared against main 92e86d4 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 393 0.24 25,406/35,840/35,840 us 🔴 +10.7% / 🔴 +160.2%
🔴 bs=100 sw=10 sl=64 812 0.496 116,460/162,052/162,052 us 🔴 +26.1% / 🔴 +74.0%
🔴 bs=1000 sw=10 sl=64 962 0.587 1,030,032/1,171,741/1,171,741 us 🔴 +6.5% / 🔴 +33.0%
Baseline details

Latest main 92e86d4 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 393 tuples/sec 412 tuples/sec 941.58 tuples/sec -4.6% -58.3%
bs=10 sw=10 sl=64 MB/s 0.24 MB/s 0.251 MB/s 0.575 MB/s -4.4% -58.2%
bs=10 sw=10 sl=64 p50 25,406 us 22,958 us 10,991 us +10.7% +131.2%
bs=10 sw=10 sl=64 p95 35,840 us 35,614 us 13,776 us +0.6% +160.2%
bs=10 sw=10 sl=64 p99 35,840 us 35,614 us 16,872 us +0.6% +112.4%
bs=100 sw=10 sl=64 throughput 812 tuples/sec 876 tuples/sec 1,264 tuples/sec -7.3% -35.8%
bs=100 sw=10 sl=64 MB/s 0.496 MB/s 0.535 MB/s 0.772 MB/s -7.3% -35.7%
bs=100 sw=10 sl=64 p50 116,460 us 112,344 us 86,073 us +3.7% +35.3%
bs=100 sw=10 sl=64 p95 162,052 us 128,552 us 93,135 us +26.1% +74.0%
bs=100 sw=10 sl=64 p99 162,052 us 128,552 us 111,124 us +26.1% +45.8%
bs=1000 sw=10 sl=64 throughput 962 tuples/sec 966 tuples/sec 1,304 tuples/sec -0.4% -26.2%
bs=1000 sw=10 sl=64 MB/s 0.587 MB/s 0.59 MB/s 0.796 MB/s -0.5% -26.3%
bs=1000 sw=10 sl=64 p50 1,030,032 us 1,033,734 us 841,986 us -0.4% +22.3%
bs=1000 sw=10 sl=64 p95 1,171,741 us 1,100,328 us 881,107 us +6.5% +33.0%
bs=1000 sw=10 sl=64 p99 1,171,741 us 1,100,328 us 918,208 us +6.5% +27.6%
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,509.51,200,128000,393,0.240,25406.49,35840.31,35840.31
1,100,10,64,20,2462.37,2000,1280000,812,0.496,116459.87,162052.34,162052.34
2,1000,10,64,20,20800.58,20000,12800000,962,0.587,1030031.74,1171741.43,1171741.43

This branch has not been deployed

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

Labels

common refactor Refactor the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Declare the chart output contract once for the Python visualization operators

3 participants