Skip to content

test(workflow-operator): add unit tests for ImageTaskCodegen - #6659

Closed
Ma77Ball wants to merge 8 commits into
apache:mainfrom
Ma77Ball:test/image-task-codegen-spec
Closed

test(workflow-operator): add unit tests for ImageTaskCodegen#6659
Ma77Ball wants to merge 8 commits into
apache:mainfrom
Ma77Ball:test/image-task-codegen-spec

Conversation

@Ma77Ball

@Ma77Ball Ma77Ball commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

  • Add common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/codegen/ImageTaskCodegenSpec.scala, a new ScalaTest spec for ImageTaskCodegen, which previously had no dedicated unit tests.
  • Cover the nine-task mapping.
  • Cover image-only vs image-plus-prompt payload branches including the zero-shot label guard, and all parse branches.
  • Confirm snippet determinism across contexts and no leakage of ctx string values.

Any related issues, documentation, discussions?

Closes: #6658

How was this PR tested?

  • Run: sbt "WorkflowOperator/testOnly *ImageTaskCodegenSpec", expect all 15 tests passing.
  • Test-only change; no production code is modified.

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

Co-authored with Claude Opus 4.8 in compliance with ASF

@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:

  • No candidates found from git blame history.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 964e915 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 552 0.337 17,406/27,952/27,952 us 🔴 +9.3% / 🔴 +73.4%
🔴 bs=100 sw=10 sl=64 1,162 0.709 82,135/126,238/126,238 us 🔴 +5.7% / 🟢 -19.0%
bs=1000 sw=10 sl=64 1,345 0.821 739,725/792,238/792,238 us ⚪ within ±5% / 🟢 +31.5%
Baseline details

Latest main 964e915 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 552 tuples/sec 566 tuples/sec 760.2 tuples/sec -2.5% -27.4%
bs=10 sw=10 sl=64 MB/s 0.337 MB/s 0.346 MB/s 0.464 MB/s -2.6% -27.4%
bs=10 sw=10 sl=64 p50 17,406 us 16,832 us 12,781 us +3.4% +36.2%
bs=10 sw=10 sl=64 p95 27,952 us 25,575 us 16,118 us +9.3% +73.4%
bs=10 sw=10 sl=64 p99 27,952 us 25,575 us 19,074 us +9.3% +46.5%
bs=100 sw=10 sl=64 throughput 1,162 tuples/sec 1,173 tuples/sec 988.68 tuples/sec -0.9% +17.5%
bs=100 sw=10 sl=64 MB/s 0.709 MB/s 0.716 MB/s 0.603 MB/s -1.0% +17.5%
bs=100 sw=10 sl=64 p50 82,135 us 82,405 us 101,448 us -0.3% -19.0%
bs=100 sw=10 sl=64 p95 126,238 us 119,411 us 107,410 us +5.7% +17.5%
bs=100 sw=10 sl=64 p99 126,238 us 119,411 us 120,258 us +5.7% +5.0%
bs=1000 sw=10 sl=64 throughput 1,345 tuples/sec 1,340 tuples/sec 1,023 tuples/sec +0.4% +31.5%
bs=1000 sw=10 sl=64 MB/s 0.821 MB/s 0.818 MB/s 0.624 MB/s +0.4% +31.5%
bs=1000 sw=10 sl=64 p50 739,725 us 743,371 us 983,888 us -0.5% -24.8%
bs=1000 sw=10 sl=64 p95 792,238 us 788,437 us 1,030,100 us +0.5% -23.1%
bs=1000 sw=10 sl=64 p99 792,238 us 788,437 us 1,056,513 us +0.5% -25.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,362.04,200,128000,552,0.337,17406.30,27951.71,27951.71
1,100,10,64,20,1720.65,2000,1280000,1162,0.709,82134.90,126238.26,126238.26
2,1000,10,64,20,14868.02,20000,12800000,1345,0.821,739725.47,792237.73,792237.73

@codecov-commenter

codecov-commenter commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.68%. Comparing base (964e915) to head (27734ba).
⚠️ Report is 12 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6659      +/-   ##
============================================
- Coverage     74.96%   74.68%   -0.28%     
- Complexity     3446     3450       +4     
============================================
  Files          1160     1157       -3     
  Lines         45795    45626     -169     
  Branches       5070     5027      -43     
============================================
- Hits          34328    34074     -254     
- Misses         9824     9902      +78     
- Partials       1643     1650       +7     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 76.76% <ø> (ø) Carriedforward from c154e3c
amber 67.20% <ø> (+<0.01%) ⬆️
computing-unit-managing-service 20.49% <ø> (ø)
config-service 66.66% <ø> (ø)
file-service 67.21% <ø> (ø)
frontend 78.29% <ø> (-0.55%) ⬇️ Carriedforward from c154e3c
notebook-migration-service 78.94% <ø> (ø)
pyamber 91.83% <ø> (-0.32%) ⬇️ Carriedforward from c154e3c
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.

@aglinxinyuan

Copy link
Copy Markdown
Contributor

Please fix the scalafmtCheck.

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 a dedicated ScalaTest spec for the Hugging Face ImageTaskCodegen to validate its handled task set and to regression-test key payload/parse snippet branches (including determinism and “no CodegenContext string leakage”).

Changes:

  • Introduce ImageTaskCodegenSpec covering the nine-task mapping and core payload/parse branches.
  • Add regression checks for snippet determinism across CodegenContext values and for absence of raw context-string inlining.

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Matthew B. <mgball@uci.edu>

@aglinxinyuan aglinxinyuan 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!

@aglinxinyuan
aglinxinyuan enabled auto-merge July 20, 2026 22:24
auto-merge was automatically disabled July 20, 2026 22:25

Head branch was pushed to by a user without write access

@aglinxinyuan

Copy link
Copy Markdown
Contributor

This test is failing the CI.

@Ma77Ball

Copy link
Copy Markdown
Contributor Author

I had to lint the branch. It should work now.

@Ma77Ball Ma77Ball closed this Jul 22, 2026
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.

Add unit test coverage for ImageTaskCodegen

4 participants