fix(amber): register EvaluatedValue in ControlReturn's sealed oneof

[eugenegujing]

What changes were proposed in this PR?

Register EvaluatedValue as a member of ControlReturn's sealed oneof (EvaluatedValue evaluatedValue = 53; in the worker-responses range of controlreturns.proto), add two Python test files that pin the fix and the underlying invariant, and add a Jackson mix-in in JSONUtils.scala that keeps the scalapb sealed-oneof helper methods out of the websocket JSON (see the follow-up section below).

Why this is a bug. The two proto files disagree about EvaluatePythonExpression's reply type: workerservice.proto declares the worker's reply as EvaluatedValue, but every worker reply must travel inside ControlReturn's sealed oneof, and that oneof only registers the coordinator-side wrapper EvaluatePythonExpressionResponse (the reply type of the coordinator's RPC, which is repeated EvaluatedValue); EvaluatedValue itself is defined in the same file but never joined the oneof, so the declared worker reply has no wire slot.

The failure is silent. On the Python engine, set_one_of assigns by the snake_case field name derived from the type name; assigning a name that is not a oneof field raises nothing on a betterproto dataclass, and serialization ignores it, so the worker's reply is packed into an empty ControlReturn: bytes(set_one_of(ControlReturn, EvaluatedValue(...))) is b'' and get_one_of returns None, with no exception and no log anywhere. The Python worker's handler does produce the correct EvaluatedValue; it is lost at the packing step, so the coordinator's Future.collect over worker replies can never receive a real value.

Why fix it in the proto. The bug lives in the contract, not in either engine's code: both engines' packing/receiving logic is correct under the assumption that the declared reply type is registered. Registering the type restores that assumption, both engines regenerate their bindings from the shared proto (generated bindings are not checked in), and no handler code changes on either side. The alternative (changing workerservice.proto to reply with the wrapper type) would touch handlers in both engines for no additional benefit.

Follow-up: keep the sealed-oneof helper methods out of the websocket JSON. Joining the sealed oneof makes the generated Scala EvaluatedValue extend the ControlReturn trait, which carries scalapb's helper methods isEmpty/isDefined. Jackson's getter scan then serialized them as "empty"/"defined" fields in the websocket JSON (EvaluatedValue is embedded in the PythonExpressionEvaluateResponse websocket event), and deserialization rejects them because the constructor only knows value/attributes. This is exactly the round-trip failure TexeraWebSocketEventSpec caught on the first CI run of this PR. The fix adds a GeneratedSealedOneofMixin with @JsonIgnore on both methods and registers it on JSONUtils.objectMapper against the scalapb.GeneratedSealedOneof interface, so every current and future sealed-oneof member is covered and the JSON wire format is byte-identical to main. Alternatives were rejected: disabling FAIL_ON_UNKNOWN_PROPERTIES globally would hide real deserialization bugs and leave the junk fields on the wire; loosening the spec's expectations would legitimize the junk fields; a per-class mix-in would leave the next sealed-oneof member to hit the same failure.

Not in scope. The Scala worker's evaluatePythonExpression remains a ??? stub (DataProcessorRPCHandlerInitializer.scala), and the coordinator fans the request out to all workers of the operator, so evaluating against an operator with Scala workers still fails on the stub; that is pre-existing behavior independent of this fix.

Any related issues, documentation, discussions?

Fixes #7924

How was this PR tested?

Two new test files were added under amber/src/test/python:

• core/util/proto/test_set_one_of.py — regression tests pinning this bug: EvaluatedValue survives set_one_of/get_one_of, and a full wire round-trip produces non-empty bytes that parse back to the original value. Rollback-verified: with the proto fix removed and bindings regenerated from the original proto, both tests fail exactly on the empty-bytes/None symptoms; with the fix restored they pass.
• core/architecture/rpc/test_reply_types_registered.py — invariant tests covering the whole bug class: every reply type declared by WorkerServiceStub (21 RPCs) and CoordinatorServiceStub (18 RPCs) must be a registered ControlReturn oneof member (the assertion message names any missing type and points to controlreturns.proto), and every registered member must survive a real set_one_of/get_one_of round-trip, which also pins the field-naming convention set_one_of depends on; non-empty guards prevent the reflection from silently passing if the generated-code layout ever changes. Rollback-verified: on the unfixed proto the invariant test fails precisely on WorkerServiceStub.evaluate_python_expression -> EvaluatedValue, so this class of bug is caught by CI at the PR that introduces it.

For the Jackson mix-in: TexeraWebSocketEventSpec goes from 9/10 (failing on the round-trip test with Unrecognized field "empty", matching this PR's first CI run) to 10/10, rollback-verified in both directions (removing only the mix-in reproduces exactly the original failure). Serializing an EvaluatedValue through JSONUtils.objectMapper was additionally asserted to produce only value/attributes and to round-trip. No other JSON path is affected: the web layer's only type carrying a sealed-oneof member is PythonExpressionEvaluateResponse, the other sealed oneofs never reach Jackson (verified per oneof), and no code references "empty"/"defined" in JSON. Wider sweep after the mix-in: the whole WorkflowCore test module passes (656 tests) and all websocket serde specs pass (25 tests); scalafmtCheck and scalafix --check pass.

Full verification: the Python suite (pytest -m "not integration") passes with the two new files included; ruff check and ruff format --check pass; ScalaPB code generation and a full sbt compile on JDK 17 succeed, and the generated Scala ControlReturn gains the SealedValue.EvaluatedValue case so the coordinator-side Future[EvaluatedValue] typing holds.

Manual repro before/after: bytes(set_one_of(ControlReturn, EvaluatedValue(value=TypedValue(expression="1+1", value_str="2")))) returns b'' before the fix and b'\xaa\x03\n\n\x08\n\x031+1\x1a\x012' (field 53) after, with get_one_of returning the full value.

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

Co-authored by: Claude Code (Claude Fable 5)

[github-actions]

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	Already labeled — this fix is queued to backport here.

Auto-label run.

[github-actions]

Automated Reviewer Suggestions

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

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

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.74%. Comparing base (fb9f4e2) to head (ec0dad9).
⚠️ Report is 74 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7927      +/-   ##
============================================
+ Coverage     91.02%   91.74%   +0.72%     
- Complexity     4454     4630     +176     
============================================
  Files          1174     1173       -1     
  Lines         47146    47514     +368     
  Branches       5287     5319      +32     
============================================
+ Hits          42915    43594     +679     
+ Misses         2551     2291     -260     
+ Partials       1680     1629      -51     

Flag	Coverage Δ		*Carryforward flag
access-control-service	81.00% <ø> (ø)
agent-service	98.62% <ø> (ø)		Carriedforward from df5dc0d
amber	89.17% <100.00%> (+1.71%)	⬆️
computing-unit-managing-service	73.67% <ø> (ø)
config-service	86.73% <ø> (ø)
file-service	84.48% <ø> (+15.57%)	⬆️
frontend	92.59% <ø> (ø)		Carriedforward from df5dc0d
notebook-migration-service	79.13% <ø> (-4.61%)	⬇️
pyamber	97.57% <ø> (ø)
workflow-compiling-service	77.19% <ø> (ø)

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

⚠️ Benchmark changes need a look

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

Compared against main 6b8027e 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	712	0.435	13,818/16,373/16,373 us	🔴 -9.4% / 🔴 +20.3%
⚪	bs=100 sw=10 sl=64	1,568	0.957	60,015/90,235/90,235 us	⚪ within ±5% / 🟢 +39.1%
🔴	bs=1000 sw=10 sl=64	1,945	1.187	508,130/606,944/606,944 us	🔴 +8.2% / 🟢 +68.3%

Baseline details

Latest main 6b8027e from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	712 tuples/sec	786 tuples/sec	863.81 tuples/sec	-9.4%	-17.6%
bs=10 sw=10 sl=64	MB/s	0.435 MB/s	0.48 MB/s	0.527 MB/s	-9.4%	-17.5%
bs=10 sw=10 sl=64	p50	13,818 us	12,841 us	11,488 us	+7.6%	+20.3%
bs=10 sw=10 sl=64	p95	16,373 us	17,661 us	14,067 us	-7.3%	+16.4%
bs=10 sw=10 sl=64	p99	16,373 us	17,661 us	17,591 us	-7.3%	-6.9%
bs=100 sw=10 sl=64	throughput	1,568 tuples/sec	1,600 tuples/sec	1,127 tuples/sec	-2.0%	+39.1%
bs=100 sw=10 sl=64	MB/s	0.957 MB/s	0.977 MB/s	0.688 MB/s	-2.0%	+39.1%
bs=100 sw=10 sl=64	p50	60,015 us	59,840 us	89,893 us	+0.3%	-33.2%
bs=100 sw=10 sl=64	p95	90,235 us	89,913 us	96,506 us	+0.4%	-6.5%
bs=100 sw=10 sl=64	p99	90,235 us	89,913 us	104,915 us	+0.4%	-14.0%
bs=1000 sw=10 sl=64	throughput	1,945 tuples/sec	1,955 tuples/sec	1,156 tuples/sec	-0.5%	+68.3%
bs=1000 sw=10 sl=64	MB/s	1.187 MB/s	1.193 MB/s	0.705 MB/s	-0.5%	+68.3%
bs=1000 sw=10 sl=64	p50	508,130 us	508,582 us	886,448 us	-0.1%	-42.7%
bs=1000 sw=10 sl=64	p95	606,944 us	560,750 us	930,093 us	+8.2%	-34.7%
bs=1000 sw=10 sl=64	p99	606,944 us	560,750 us	963,104 us	+8.2%	-37.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,280.88,200,128000,712,0.435,13817.81,16373.10,16373.10
1,100,10,64,20,1275.46,2000,1280000,1568,0.957,60014.61,90234.61,90234.61
2,1000,10,64,20,10281.27,20000,12800000,1945,1.187,508129.72,606943.58,606943.58