Skip to content

feat(workflow-operator): keep the row when a Hugging Face text cell is empty - #7569

Open
kz930 wants to merge 1 commit into
apache:mainfrom
kz930:fix/7549-hf-empty-text
Open

feat(workflow-operator): keep the row when a Hugging Face text cell is empty#7569
kz930 wants to merge 1 commit into
apache:mainfrom
kz930:fix/7549-hf-empty-text

Conversation

@kz930

@kz930 kz930 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Sentiment Analysis, Spam SMS Detection and Text Summarization each passed their text column straight to a tokenizer or pipeline. An empty cell arrives as None, which transformers rejects with ValueError: You need to specify either text or text_target for the first two and ValueError: text input must be of type str ... for the third, ending the run.

An empty value is ordinary input here. A blank CSV cell arrives as null, since univocity returns null for an empty field and AttributeTypeUtils.parseField passes it through by design.

All three now yield the row with their result attributes left empty. That follows what the Hugging Face inference operator does with a row it cannot process: HuggingFaceCodegenBase appends the error to that row's results and continues rather than dropping it.

The visualization operators drop such rows instead, with dropna(subset=[...]), but their output is a chart, where a missing row costs nothing. These three add columns to each input row, so dropping would take the user's row out of the output along with the value the model had nothing to say about. Whitespace-only text takes the same path, for the same reason.

Any related issues, documentation, discussions?

Closes #7549

How was this PR tested?

Each of the three specs gains a case asserting the generated Python guards the empty cell before the value reaches the tokenizer or pipeline. All three fail on the previous behavior, 79 passed / 3 failed before the change and 82 / 0 after.

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

Generated-by: Claude Code (Claude Opus 5)

@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 11, 2026
@github-actions
github-actions Bot requested a review from xuang7 August 11, 2026 20:45
@github-actions

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
release/v1.2 Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7.

Auto-label run.

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

@codecov-commenter

codecov-commenter commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.87%. Comparing base (e878df3) to head (815365c).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7569      +/-   ##
============================================
- Coverage     87.88%   87.87%   -0.02%     
+ Complexity     4276     4271       -5     
============================================
  Files          1176     1176              
  Lines         47018    47022       +4     
  Branches       5245     5245              
============================================
- Hits          41323    41320       -3     
- Misses         3972     3974       +2     
- Partials       1723     1728       +5     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 98.62% <ø> (ø) Carriedforward from e878df3
amber 82.84% <100.00%> (-0.04%) ⬇️
computing-unit-managing-service 60.38% <ø> (ø)
config-service 65.97% <ø> (ø)
file-service 69.05% <ø> (ø)
frontend 89.94% <ø> (ø) Carriedforward from e878df3
notebook-migration-service 78.89% <ø> (ø)
pyamber 97.52% <ø> (ø) Carriedforward from e878df3
workflow-compiling-service 26.31% <ø> (ø)

*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.

…s empty

Sentiment Analysis, Spam SMS Detection and Text Summarization each passed their
text column straight to a tokenizer or pipeline. An empty cell arrives as None,
which transformers rejects with `ValueError: You need to specify either text or
text_target` for the first two and `ValueError: text input must be of type str
...` for the third, ending the run.

An empty value is ordinary input here. A blank CSV cell arrives as null:
univocity returns null for an empty field, and AttributeTypeUtils.parseField
passes it through by design.

All three now yield the row with their result attributes left empty. That
follows what the Hugging Face inference operator does with a row it cannot
process: HuggingFaceCodegenBase appends the error to that row's results and
continues rather than dropping it. The visualization operators drop such rows
instead, with `dropna(subset=[...])`, but their output is a chart, where a
missing row costs nothing; these three add columns to each input row, so
dropping would take the user's row out of the output along with the value the
model had nothing to say about.

Whitespace-only text takes the same path, since the models have nothing to say
about it either.

Each spec gains the case; all three fail on the previous behavior.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@kz930
kz930 force-pushed the fix/7549-hf-empty-text branch from 0f1959b to 815365c Compare August 11, 2026 20:51
@kz930 kz930 changed the title fix(workflow-operator): keep the row when a Hugging Face text cell is empty feat(workflow-operator): keep the row when a Hugging Face text cell is empty Aug 11, 2026
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main e878df3 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 401 0.245 24,243/33,650/33,650 us 🔴 +13.6% / 🔴 +110.4%
🔴 bs=100 sw=10 sl=64 772 0.471 127,846/157,798/157,798 us 🔴 +17.6% / 🔴 +46.0%
🔴 bs=1000 sw=10 sl=64 873 0.533 1,138,796/1,208,389/1,208,389 us 🔴 +6.9% / 🔴 +16.4%
Baseline details

Latest main e878df3 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 401 tuples/sec 436 tuples/sec 784.3 tuples/sec -8.0% -48.9%
bs=10 sw=10 sl=64 MB/s 0.245 MB/s 0.266 MB/s 0.479 MB/s -7.9% -48.8%
bs=10 sw=10 sl=64 p50 24,243 us 21,343 us 12,590 us +13.6% +92.6%
bs=10 sw=10 sl=64 p95 33,650 us 33,255 us 15,991 us +1.2% +110.4%
bs=10 sw=10 sl=64 p99 33,650 us 33,255 us 18,694 us +1.2% +80.0%
bs=100 sw=10 sl=64 throughput 772 tuples/sec 837 tuples/sec 1,002 tuples/sec -7.8% -23.0%
bs=100 sw=10 sl=64 MB/s 0.471 MB/s 0.511 MB/s 0.612 MB/s -7.8% -23.0%
bs=100 sw=10 sl=64 p50 127,846 us 118,157 us 101,285 us +8.2% +26.2%
bs=100 sw=10 sl=64 p95 157,798 us 134,168 us 108,068 us +17.6% +46.0%
bs=100 sw=10 sl=64 p99 157,798 us 134,168 us 118,235 us +17.6% +33.5%
bs=1000 sw=10 sl=64 throughput 873 tuples/sec 923 tuples/sec 1,030 tuples/sec -5.4% -15.2%
bs=1000 sw=10 sl=64 MB/s 0.533 MB/s 0.563 MB/s 0.629 MB/s -5.3% -15.2%
bs=1000 sw=10 sl=64 p50 1,138,796 us 1,077,593 us 991,882 us +5.7% +14.8%
bs=1000 sw=10 sl=64 p95 1,208,389 us 1,130,577 us 1,038,496 us +6.9% +16.4%
bs=1000 sw=10 sl=64 p99 1,208,389 us 1,130,577 us 1,068,265 us +6.9% +13.1%
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,498.17,200,128000,401,0.245,24243.43,33649.50,33649.50
1,100,10,64,20,2589.53,2000,1280000,772,0.471,127845.70,157798.36,157798.36
2,1000,10,64,20,22908.23,20000,12800000,873,0.533,1138795.85,1208388.76,1208388.76

@xuang7
xuang7 removed their request for review August 11, 2026 21:04
@xuang7 xuang7 removed fix release/v1.2 back porting to release/v1.2 labels Aug 11, 2026
@kz930

kz930 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@carloea2, @aglinxinyuan May you take a look at this?

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

Looks good to me.

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.

Sentiment Analysis, Spam SMS Detection and Text Summarization fail on an empty text cell

5 participants