Conversation
Some operators need an input a schema alone cannot produce. A join needs two tables that share a key. A dictionary matcher needs a dictionary whose entries appear in the column it reads. Deriving those from annotations would mean teaching the generator what each operator means. These are written by hand instead, one entry per operator, and the entry says why the generator could not do it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8363 +/- ##
============================================
- Coverage 94.11% 93.32% -0.79%
- Complexity 4811 4904 +93
============================================
Files 1197 1210 +13
Lines 48813 49647 +834
Branches 5906 6035 +129
============================================
+ Hits 45939 46332 +393
- Misses 1420 1798 +378
- Partials 1454 1517 +63
*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:
|
The behaviour is what a reader needs; the change that introduced it is what a blame is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The handler wrote a three-column table of its own: id, name, score. Those are the first three columns of the shared one, so the table was never the reason this operator is curated. Its config is: `attributes` is not declared required, the auto tier starts it empty as the UI does, and the operator refuses an empty list. Pinning that one row is all it needs. Dropping the table puts the operator on the same thirty-four columns every other one reads, where a projection has something to leave out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
The curated configurations look good.
Every source the fixture held was a number, and a number is the one source where Python and the engine cannot disagree: `bool(x)` is false exactly when x is zero, which is what the engine's `x != 0` says. The disagreement lives in the string column the fixture had none of, where Python answers true for "false" and for "0". The values are the ones both sides accept, since text the engine refuses would end the run before there is anything to compare. That half is pinned against `AttributeTypeUtils` in the operator's own spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
The new boolean cases cover the earlier mismatch. Looks good.
These four write their own tables, so the shared table's renaming does not reach them. Each renames the column its own configuration points at, which is what puts the escaping question to the code it emits. Three of the headers here also claimed their rows compare by position. None of those operators declares itself order-sensitive, so the comparator lex-sorts both sides first, and the claim is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
b97eab5 to
082a6c3
Compare
Regex, Keyword Search and Type Casting each gained the case their table could not put: an integer column and a boolean one, whose text a hole changes; a column named by two casting units, which the executor collapses to the last; a 64-bit value past the Int bounds; a number read as a timestamp; and text NumberFormat reads where Python's int refuses. Aggregate gained a leading empty string, which is the only value that tells the two spellings of CONCAT apart: a leading null reads the same either way, and the canonical table holds no empty strings. Keyword Search also gained a case-sensitive run over a column carrying one word in three casings. Its base table is lower-case throughout, so sweeping the flag there decided nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It restated its own signature and listed a codec table that lives in TupleIO. The reasons a reader cannot derive stay. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@carloea2 this moved after your approval: the curated tables got hostile column names, and they are now asked what a rendered column looks like. Would you take another look when you have a moment? |
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
The curated configurations cover the cases the schema cannot generate. Looks good.
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
The If test only checks the true output. It never sends a condition state, so the false route can be broken and this test still passes. Please add a case that selects false and verify the rows go to the false output only.
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
The new verifier marks Keyword Search as passing, but punctuation already gives different results. With text perfect. and query perfect in case sensitive mode, native does not match because it keeps perfect. as one token. The exported regex matches perfect and keeps the row. Please add this case and do not certify the operator until both paths agree.
The comment sat above AggregateTransformHandler rather than the handler it describes, and it read as a choice rather than a limit. If's route is decided by a State message on the Condition port, and the harness has no State channel, so a fixture can only reach the engine's default, True. Say that, and say where the False route is covered instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The route is chosen by a State message on the Condition port and the harness writes rows per port, so a fixture can only reach the engine's default, which is True. The False route is covered where it can be: |
The note explaining the punctuation-free rows described an analyzer that tokenized on whitespace, which stopped being true when CaseSensitiveAnalyzer moved to the StandardTokenizer: a term next to punctuation is a term to the engine and to a word-boundary regex alike. Say what the two do part on instead, a word punctuation belongs inside, and let the case-sensitive scenario carry a row that ends its term in a period. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
|
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
My earlier claim about perfect. was wrong. The native analyzer uses StandardTokenizer, which removes the trailing period. The updated fixture covers this case. I withdraw that finding and approve.
Sklearn Prediction and Sklearn Testing read a fitted model on port 0, so no auto fixture can run them. The handlers fit a DecisionTree in Python on the table they write to port 1 and write it to port 0 the way Tuple.cast_to_schema writes a model into a BINARY field. The last row is held out of the fit and lands on the wrong side, so no score is perfect and the averaging behind it is actually compared. Testing also runs a regressor on a numeric target, which covers the isRegression branch. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s runs The fixture read UDF_PYTHON_PATH itself. PyOpExecHarness.resolvePython is the one place that says which interpreter the harness runs, and the model has to be pickled by the same one that unpickles it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
3 participants
What changes were proposed in this PR?
Some operators need an input a schema alone cannot produce. A join needs two
tables that share a key. A dictionary matcher needs a dictionary whose entries
appear in the column it reads. Deriving those from annotations would mean
teaching the generator what each operator means.
These are written by hand instead, one entry per operator, and the entry says
why the generator could not do it.
Any related issues, documentation, discussions?
Part of #8325, 9 of 27; that issue lists the set in order.
Closes #8413, 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)