Repository navigation
Conversation
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>
Contributor
Automated Reviewer SuggestionsBased on the
|
This was referenced Sep 2, 2026
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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8358 +/- ##
============================================
- Coverage 93.50% 93.02% -0.49%
+ Complexity 4882 4878 -4
============================================
Files 1220 1221 +1
Lines 50788 51146 +358
Branches 6262 6299 +37
============================================
+ Hits 47489 47578 +89
- Misses 1746 2018 +272
+ Partials 1553 1550 -3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
carloea2
approved these changes
Sep 4, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
The standalone runner looks good.
kz930
marked this pull request as draft
September 4, 2026 17:12
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
approved these changes
Sep 4, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
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>
kz930
force-pushed
the
feat/verify-run-generated-script
branch
from
September 9, 2026 06:50
e28d256 to
dd36970
Compare
carloea2
reviewed
Sep 9, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
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>
kz930
force-pushed
the
feat/verify-run-generated-script
branch
from
September 9, 2026 23:42
7c3d0e4 to
3e87f6b
Compare
…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>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@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
approved these changes
Sep 18, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
Empty inputs keep their declared columns, and integer values are read without rounding. Looks good.
carloea2
suggested changes
Sep 18, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
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
suggested changes
Sep 18, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
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>
Contributor
Author
|
Binary is fixed in |
kz930
added a commit
to Nicoleee1108/texera_workflow_to_py
that referenced
this pull request
Sep 19, 2026
carloea2
approved these changes
Sep 19, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
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>
apache#8488 is closed: the engine hands an operator a table with no columns when a port carried no rows, so rebuilding them from the sidecar would make the generated script differ from the run it is compared against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
carloea2
reviewed
Sep 23, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
Two timestamp mismatches reproduced from the generated harness code, with normal-range and microsecond inputs as passing controls. Java Timestamp was checked separately; I did not run the full Scala harness.
…ts both reach The harness read every TIMESTAMP column through pd.to_datetime, whose nanoseconds reach only 1677 to 2262, so a year-2500 input failed before the operator ran. Java's Timestamp holds it. A column outside that window is now read cell by cell at microseconds. On the way out, %f wrote six digits of the fraction where Timestamp.toString writes nine, so an unchanged value could disagree with the run because of the harness alone. The fraction is now built from the nanoseconds. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
read_json finds no column names in a file with no rows, so the script was handed a frame of no columns. An exported script never sees one: a filter that matches nothing and a header-only CSV both leave the columns in place, and each operator's code is handed that frame. Without them 21 operators that handle an empty table raised KeyError on the script side only, a failure the test made rather than one the export has. The columns are rebuilt from the sidecar again. The earlier removal took the engine's columnless empty table as the reference for this side too, but the engine is what Path A stands for, not the script. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The worker's ArrowTableTupleProvider unpickles a BINARY cell that starts with the cast's pickle marker, so a model column reaches the next operator as a model. The script's prologue decoded the base64 and stopped there, so an operator reading a model port was handed bytes. The prologue now makes the same test and slice, so the script gets the object the run gets and any other bytes stay bytes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The pickled-cell test used a pickled list in place of a model. It now pickles a fitted DecisionTreeClassifier with the interpreter the script runs and has the script call predict on it, which is the call that raised AttributeError when the prologue handed over bytes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The JSONL an output is written to carries values and not their dtypes, so a float32 and a float64 write the same text and a zoned timestamp loses its zone on the way. Both still reach the next operator as what they are. The script now writes each column's dtype beside the output, taken before the encoding rewrites the frame, for a runner to check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…erpreter it falls back to Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
The other side of the comparison.
StandaloneRunnerwrites the script theoperator'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.
HarnessSpeccovers all three pieces on one operator whose answer is shortenough 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
StandaloneRunnerreuses thePythonWorkerPoolthat 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)