Skip to content

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

Open
kz930 wants to merge 3 commits into
apache:mainfrom
kz930:feat/verify-compare-two-answers
Open

test(verify): compare what the two paths produced#8359
kz930 wants to merge 3 commits into
apache:mainfrom
kz930:feat/verify-compare-two-answers

Conversation

@kz930

@kz930 kz930 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Two frames agree when they hold the same rows in the same order with the same
values, and the comparison says which of those failed rather than that they
differ. A float is compared within a tolerance, since the two paths reach the
same number by different arithmetic.

A visualization has no frame to compare. What it emits is a figure, and the
meaning of a figure is in the numbers behind it, so the figures are compared
number by number; a plot that renders as an image is compared as the HTML it
emits instead.

Any related issues, documentation, discussions?

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

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)

Two frames agree when they hold the same rows in the same order with the same
values, and the comparison says which of those failed rather than that they
differ. A float is compared within a tolerance, since the two paths reach the
same number by different arithmetic.

A visualization has no frame to compare. What it emits is a figure, and the
meaning of a figure is in the numbers behind it, so the figures are compared
number by number; a plot that renders as an image is compared as the HTML it
emits instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added feature platform Non-amber Scala service paths labels Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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.

@codecov-commenter

codecov-commenter commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.11%. Comparing base (7976744) to head (4ea66b3).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8359   +/-   ##
=========================================
  Coverage     94.11%   94.11%           
  Complexity     4811     4811           
=========================================
  Files          1197     1197           
  Lines         48813    48814    +1     
  Branches       5906     5906           
=========================================
+ Hits          45939    45940    +1     
  Misses         1420     1420           
  Partials       1454     1454           
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø)
agent-service 99.32% <ø> (ø) Carriedforward from 9111b31
amber 90.08% <ø> (ø) Carriedforward from 9111b31
computing-unit-managing-service 73.67% <ø> (ø)
config-service 87.12% <ø> (+0.12%) ⬆️
file-service 87.91% <ø> (ø)
frontend 96.86% <ø> (ø) Carriedforward from 9111b31
notebook-migration-service 79.31% <ø> (ø)
pyamber 98.47% <ø> (ø) Carriedforward from 9111b31
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 9111b31

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

`read_json` infers a type per file, so a column one side wrote as "6" and the
other wrote as "6.0" both arrive as the number 6, and a null beside the text
"nan" both arrive as NaN. Two different answers compared as one, on every
string column of every operator.

The engine writes a schema beside its output naming which columns are strings,
and both sides are now read that way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kz930 added a commit to kz930/texera that referenced this pull request Sep 3, 2026
`astype(str)` gets three things wrong against `toString`: an empty cell
renders as the text "nan", a column holding one has become a float by then so
6 reads "6.0", and a boolean capitalises where the executor writes it lower.

The comparison could not see any of it until apache#8359 stopped inferring a type
per file, which is why a cast to STRING has been passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature platform Non-amber Scala service paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants