Skip to content

test(workflow-operator): add unit test coverage for ML scorer and sort enums#6044

Merged
xuang7 merged 2 commits into
apache:mainfrom
aglinxinyuan:test-ml-scorer-sort-enums
Jul 1, 2026
Merged

test(workflow-operator): add unit test coverage for ML scorer and sort enums#6044
xuang7 merged 2 commits into
apache:mainfrom
aglinxinyuan:test-ml-scorer-sort-enums

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Pin behavior of three previously-untested ML-scorer / sort enums in common/workflow-operator. No production-code changes.

Spec Source enum Tests
classificationMetricsFncSpec classificationMetricsFnc 2
regressionMetricsFncSpec regressionMetricsFnc 2
SortPreferenceSpec SortPreference 2

Behavior pinned

Surface Contract
wire mapping classificationMetricsFnc/regressionMetricsFnc @JsonValue names (Accuracy/Precision Score/…, MSE/RMSE/…); constant counts
SortPreference ASC/DESC constants (no @JsonValue, so serialized by constant name)
Jackson round-trip serialize + deserialize back to the constant

Any related issues, documentation, discussions?

Part of the ongoing workflow-operator unit-test coverage effort.

How was this PR tested?

  • sbt "WorkflowOperator/testOnly *classificationMetricsFncSpec *regressionMetricsFncSpec *SortPreferenceSpec" — 6 tests, all green
  • sbt "WorkflowOperator/Test/scalafmtCheck" and sbt "WorkflowOperator/scalafixAll --check" — clean
  • CI to confirm

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

Generated-by: Claude Code (Opus 4.8 [1M context])

Copilot AI review requested due to automatic review settings July 1, 2026 01:25
@github-actions github-actions Bot added the common label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

  • No candidates found from git blame history.

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.

Pull request overview

Adds ScalaTest unit coverage in common/workflow-operator to pin the serialization/wire-contract behavior of three existing Java enums used by ML scorer and sort operators, without changing production logic.

Changes:

  • Add SortPreferenceSpec validating enum constant set and Jackson round-trip via constant names.
  • Add classificationMetricsFncSpec validating @JsonValue wire names, constant count, and Jackson round-trip.
  • Add regressionMetricsFncSpec validating @JsonValue wire names, constant count, and Jackson round-trip.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sort/SortPreferenceSpec.scala New unit tests for SortPreference constant set and Jackson serialization/deserialization.
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/Scorer/regressionMetricsFncSpec.scala New unit tests pinning regressionMetricsFnc wire names (@JsonValue) and Jackson round-trip.
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/machineLearning/Scorer/classificationMetricsFncSpec.scala New unit tests pinning classificationMetricsFnc wire names (@JsonValue) and Jackson round-trip.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter

codecov-commenter commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.93%. Comparing base (434bcae) to head (576a3d1).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6044      +/-   ##
============================================
- Coverage     56.93%   56.93%   -0.01%     
+ Complexity     3026     3024       -2     
============================================
  Files          1129     1129              
  Lines         43794    43794              
  Branches       4743     4743              
============================================
- Hits          24936    24933       -3     
- Misses        17384    17385       +1     
- Partials       1474     1476       +2     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 44.59% <ø> (ø) Carriedforward from 2b7a6d9
amber 58.75% <ø> (-0.02%) ⬇️
computing-unit-managing-service 0.00% <ø> (ø)
config-service 52.30% <ø> (ø)
file-service 62.81% <ø> (ø)
frontend 50.12% <ø> (ø) Carriedforward from 2b7a6d9
notebook-migration-service 78.57% <ø> (ø)
pyamber 90.20% <ø> (ø) Carriedforward from 2b7a6d9
python 90.76% <ø> (ø) Carriedforward from 2b7a6d9
workflow-compiling-service 55.14% <ø> (ø)

*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

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 1 worse · ⚪ 12 noise (<±5%) · 0 without baseline

Compared against main 434bcae 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 390 0.238 25,857/32,363/32,363 us 🟢 -17.0% / 🔴 +114.7%
bs=100 sw=10 sl=64 791 0.483 125,226/143,490/143,490 us ⚪ within ±5% / 🔴 +33.4%
bs=1000 sw=10 sl=64 918 0.56 1,089,406/1,130,078/1,130,078 us ⚪ within ±5% / 🔴 +10.4%
Baseline details

Latest main 434bcae from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 390 tuples/sec 385 tuples/sec 777.62 tuples/sec +1.3% -49.8%
bs=10 sw=10 sl=64 MB/s 0.238 MB/s 0.235 MB/s 0.475 MB/s +1.3% -49.9%
bs=10 sw=10 sl=64 p50 25,857 us 24,236 us 12,612 us +6.7% +105.0%
bs=10 sw=10 sl=64 p95 32,363 us 38,972 us 15,070 us -17.0% +114.7%
bs=10 sw=10 sl=64 p99 32,363 us 38,972 us 18,360 us -17.0% +76.3%
bs=100 sw=10 sl=64 throughput 791 tuples/sec 787 tuples/sec 988.31 tuples/sec +0.5% -20.0%
bs=100 sw=10 sl=64 MB/s 0.483 MB/s 0.48 MB/s 0.603 MB/s +0.6% -19.9%
bs=100 sw=10 sl=64 p50 125,226 us 124,862 us 101,066 us +0.3% +23.9%
bs=100 sw=10 sl=64 p95 143,490 us 143,353 us 107,594 us +0.1% +33.4%
bs=100 sw=10 sl=64 p99 143,490 us 143,353 us 115,830 us +0.1% +23.9%
bs=1000 sw=10 sl=64 throughput 918 tuples/sec 921 tuples/sec 1,019 tuples/sec -0.3% -10.0%
bs=1000 sw=10 sl=64 MB/s 0.56 MB/s 0.562 MB/s 0.622 MB/s -0.4% -10.0%
bs=1000 sw=10 sl=64 p50 1,089,406 us 1,086,704 us 986,982 us +0.2% +10.4%
bs=1000 sw=10 sl=64 p95 1,130,078 us 1,133,517 us 1,028,491 us -0.3% +9.9%
bs=1000 sw=10 sl=64 p99 1,130,078 us 1,133,517 us 1,058,493 us -0.3% +6.8%
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,513.01,200,128000,390,0.238,25857.43,32362.81,32362.81
1,100,10,64,20,2529.39,2000,1280000,791,0.483,125225.69,143490.37,143490.37
2,1000,10,64,20,21782.36,20000,12800000,918,0.560,1089405.70,1130078.15,1130078.15

Per review: rename classificationMetricsFncSpec/regressionMetricsFncSpec (class + file)
to ClassificationMetricsFncSpec/RegressionMetricsFncSpec for Scala naming consistency
and test-report discoverability.
@aglinxinyuan aglinxinyuan requested a review from xuang7 July 1, 2026 04:55

@xuang7 xuang7 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.

LGTM!

@xuang7 xuang7 added this pull request to the merge queue Jul 1, 2026
Merged via the queue into apache:main with commit 2a2bac2 Jul 1, 2026
23 checks passed
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.

4 participants