test(verify): run the script the export produces by kz930 · Pull Request #8358 · apache/texera

kz930 · 2026-09-02T23:31:13Z

What changes were proposed in this PR?

The other side of the comparison. StandaloneRunner writes the script the
operator's generator emits, binds its inputs to the files the fixture wrote,
runs it, and reads the frames it leaves behind.

The script is kept where it ran, so an operator whose two answers differ can
be opened as generated rather than described second-hand.

HarnessSpec covers all three pieces on one operator whose answer is short
enough to state in full.

Any related issues, documentation, discussions?

Part of #8325, 5 of 27; that issue lists the set in order.

It does not compile until #8357 lands: that entry's build.sbt change puts workflow-operator's test sources on this module's test classpath, and StandaloneRunner reuses the PythonWorkerPool that lives there.

Closes #8410, the task this change is the whole of.

How was this PR tested?

The tests in this change cover it. The whole set is exercised together once the last piece lands: every operator run through the engine and through its generated script, and the two answers compared.

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

Generated-by: Claude Code (Opus 5)

The other side of the comparison. `StandaloneRunner` writes the script the operator's generator emits, binds its inputs to the files the fixture wrote, runs it, and reads the frames it leaves behind. The script is kept where it ran, so an operator whose two answers differ can be opened as generated rather than described second-hand. `HarnessSpec` covers all three pieces on one operator whose answer is short enough to state in full. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-02T23:31:29Z

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.

Three kinds of comment came out. A drawing of the string the code below assembles. A restatement of a branch the reader can see. And the word MVP, which dated the scope to a moment rather than stating it. What replaces them says the same thing shorter, or says what the code cannot: which cases the harness does not drive and why none of them has an operator asking for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

codecov-commenter · 2026-09-03T22:49:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.98%. Comparing base (bf356f5) to head (36f79d8).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8358      +/-   ##
============================================
- Coverage     93.50%   92.98%   -0.52%     
+ Complexity     4882     4876       -6     
============================================
  Files          1220     1221       +1     
  Lines         50788    51156     +368     
  Branches       6262     6301      +39     
============================================
+ Hits          47489    47568      +79     
- Misses         1746     2038     +292     
+ Partials       1553     1550       -3

Flag	Coverage Δ		*Carryforward flag
access-control-service	`71.78% <ø> (ø)`
agent-service	`99.32% <ø> (ø)`		Carriedforward from f4b9496
amber	`89.13% <ø> (+0.02%)`	⬆️	Carriedforward from f4b9496
computing-unit-managing-service	`55.20% <ø> (-21.95%)`	⬇️
config-service	`87.37% <ø> (+0.12%)`	⬆️
file-service	`81.53% <ø> (ø)`
frontend	`96.72% <ø> (+0.03%)`	⬆️	Carriedforward from f4b9496
notebook-migration-service	`83.73% <ø> (ø)`
pyamber	`98.47% <ø> (ø)`		Carriedforward from f4b9496
workflow-compiling-service	`74.09% <ø> (ø)`		Carriedforward from f4b9496

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

carloea2

The standalone runner looks good.

The harness wrote plotly's three modules into every rendered script, so an operator that draws with plotly and never says so still ran here and would have failed on export. The header now carries pandas plus what the operator declares, which is what the translator emits. The pooled worker keeps pre-importing plotly, since that is a cache warmer rather than a name the script can reach: each script runs in a fresh namespace, so the missing declaration still surfaces as a NameError. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

carloea2

The verification script now uses the same imports as the exported script. This looks good.

An operator that stops early on an input it cannot draw ends itself with SystemExit. Under `python script.py` nothing catches that and the interpreter exits with the code; here the catch-all below read it as a crash, so a run that succeeded was reported as a failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

carloea2

Reviewed alongside the related export and verification PRs. These findings are based on code inspection and focused Python checks, not a full Scala suite run.

pd.read_json infers a column of numeric-looking strings as a number, so a STRING column holding 001 loaded as 1 and one holding only nulls loaded as NaN. The engine path calls str() on the cell and leaves a null alone, so the two disagreed about the operator's input rather than its output. The fixture's schema sidecar already says which columns those are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…xactly The sidecar already says what each input column was declared as, and the generator now has an overload that takes it, so the script is told what a JSONL file cannot carry. `read_json` parses a column holding a null through float64, so a LONG of 9007199254740993 reached the operator as 9007199254740992 while Path A still held the tuple. The engine's own Python side refuses a float outside that window rather than accept a corrupted rendition; the harness was quietly accepting one. Only where Path A is the JVM harness: a Python operator's table is built by pandas from the same tuples, so the float is what BOTH sides see there. Writing needs two more unwrappings for the dtype that follows: an operator rebuilding rows out of an integer column hands back numpy scalars, and a nullable column carries pd.NA. Neither is a Python scalar, so both were being written into the JSONL as base64 pickles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`read_json` has no rows to read column names off a file with none in it, so it produced a frame of no columns while the engine hands the operator the port's declared ones. A Projection naming a column then raised KeyError on a table the workflow handled. The frame is rebuilt from the sidecar, dtypes included, so an empty table is the same table on both paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Twenty lines in one paragraph read as a wall. Same reasons, one short paragraph apiece. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kz930 · 2026-09-11T01:18:50Z

@carloea2 all three are fixed: the STRING columns, the empty input that lost its columns, and the LONG precision. Would you take another look when you have a moment?

carloea2

Empty inputs keep their declared columns, and integer values are read without rounding. Looks good.

carloea2

Binary input is read from JSON as base64 text and is never decoded back to bytes. The engine path gives the operator bytes. Please restore binary columns from the sidecar before running the generated code and add a binary input test.

carloea2

Boolean input types are not restored from the sidecar. A nullable boolean column can become numbers and missing floats in pandas, so the generated path receives different values from the engine path. Please restore boolean columns and test true, false, and null together.

pandas has no plain boolean column that carries a null, so read_json reads one with a hole as float64 and the script was handed 1.0 and 0.0 where the run had true and false. Seventeen operators compared unequal on that column alone. The prologue takes such a column to the nullable boolean dtype, which carries the two values and the hole, and leaves a column without a hole alone: that one arrives as bool already, and the nullable dtype would be one the run never had. The same rule the exact-integer restore follows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The engine writes a tuple through the schema, so a column it declares INTEGER leaves as an integer however the operator held it. pandas has no plain integer that carries a null, so the same column left the script as 6.0 as soon as a row was missing, and the two sides then disagreed on every value in it. Seven operators were red on that alone. The epilogue writes the declared columns as integers, hole kept, and only where the values are whole: a column the script filled with 6.5 where the run had 6 is a real disagreement, and rounding it here would report the two as equal. An operator that rebuilds its rows hands back objects rather than floats, so the values are read one at a time. The schemas come from the caller, which has the ones the run itself produced. Asking the operator to propagate a second time builds it a second physical plan, and Aggregate's two-stage one does not survive that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…gets JSON carries bytes as base64 text, which is how the fixture carries a BINARY column, and the engine decodes it before the operator sees the field. The script was handed the text: its operator held a str where the run's held bytes, and anything it did with them, a length or a decode or a digest, answered for the base64 rather than for the value. Distinct answers the same either way, which is why the column looked right on the way out. Decoding it on the way in is half of it. The writer pickles whatever is not a plain scalar, so the bytes would have left as a base64 pickle of themselves where the engine's writer writes the base64 of the value. Bytes are now written as that base64. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kz930 · 2026-09-19T04:13:58Z

Binary is fixed in f4b9496eb: the prologue decodes the column to bytes, and the writer now writes bytes as the base64 the engine's writer writes rather than as a pickle of them, which decoding alone would have produced. Boolean was already restored from the sidecar in 04f55557a, with true, false and null in one column as the test.

carloea2

The binary input and nullable boolean handling now follow the declared schema. The earlier blockers are addressed. Looks good.

…the caller A source writes a placeholder where its file should be named and leaves the naming to whoever assembles the script, so that two sources reading different files whose paths end alike do not both ask for the same one. The translator does that across a whole plan. This runner assembles a single operator and bound nothing, so every source's script stopped on a `sourceFile` that was never defined. It binds the name the source offers, there being nobody to collide with in a one-operator script. No operator spec could have caught this: they assert the text the operator emits, and the text is right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions Bot assigned kz930 Sep 2, 2026

github-actions Bot added feature platform Non-amber Scala service paths labels Sep 2, 2026

carloea2 approved these changes Sep 4, 2026

View reviewed changes

kz930 marked this pull request as draft September 4, 2026 17:12

carloea2 approved these changes Sep 4, 2026

View reviewed changes

kz930 force-pushed the feat/verify-run-generated-script branch from e28d256 to dd36970 Compare September 9, 2026 06:50

carloea2 reviewed Sep 9, 2026

View reviewed changes

Comment thread ...ling-service/src/test/scala/org/apache/texera/amber/translator/verify/StandaloneRunner.scala Outdated

kz930 force-pushed the feat/verify-run-generated-script branch from 7c3d0e4 to 3e87f6b Compare September 9, 2026 23:42

kz930 and others added 5 commits September 10, 2026 13:08

test(verify): say the reader's four options once each

06ffa18

Twenty lines in one paragraph read as a wall. Same reasons, one short paragraph apiece. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

style: wrap a line scalafmt rejects

f0616cc

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test(verify): cut the doc to what the code cannot say

f99d1ef

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Merge branch 'main' into feat/verify-run-generated-script

0f78bb1

carloea2 approved these changes Sep 18, 2026

View reviewed changes

carloea2 suggested changes Sep 18, 2026

View reviewed changes

kz930 and others added 2 commits September 18, 2026 16:45

kz930 mentioned this pull request Sep 19, 2026

test(verify): compare what the two paths produced #8359

Draft

kz930 added a commit to Nicoleee1108/texera_workflow_to_py that referenced this pull request Sep 19, 2026

merge: PR apache#8358 (updated head)

f1c5a81

carloea2 approved these changes Sep 19, 2026

View reviewed changes

Navigation Menu

test(verify): run the script the export produces - #8358

test(verify): run the script the export produces#8358
kz930 wants to merge 15 commits into
apache:mainfrom
kz930:feat/verify-run-generated-script

kz930 commented Sep 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 2, 2026

Uh oh!

codecov-commenter commented Sep 3, 2026 •

edited

Loading

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

Uh oh!

kz930 commented Sep 11, 2026 •

edited

Loading

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

kz930 commented Sep 19, 2026

Uh oh!

carloea2 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kz930 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this PR?

Any related issues, documentation, discussions?

How was this PR tested?

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

Uh oh!

github-actions Bot commented Sep 2, 2026

Automated Reviewer Suggestions

Uh oh!

codecov-commenter commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kz930 commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

kz930 commented Sep 19, 2026

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kz930 commented Sep 2, 2026 •

edited

Loading

codecov-commenter commented Sep 3, 2026 •

edited

Loading

kz930 commented Sep 11, 2026 •

edited

Loading