Skip to content

test(verify): run the source operators, and report what is covered - #8364

Draft
kz930 wants to merge 6 commits into
apache:mainfrom
kz930:feat/verify-sources-and-coverage
Draft

test(verify): run the source operators, and report what is covered#8364
kz930 wants to merge 6 commits into
apache:mainfrom
kz930:feat/verify-sources-and-coverage

Conversation

@kz930

@kz930 kz930 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

A source reads a file rather than an upstream port, so it is configured from a
file the runner writes rather than from a table, and the variants follow the
configuration: a CSV with a header and without, an encoding per charset.

Two reports come with it. One prints which operators run and which are
withheld and why, so the state of the set is a command rather than a claim.
The other parses every generated script and fails on one that a hostile column
name would break, which is a whole class of defect a comparison cannot see.

Any related issues, documentation, discussions?

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

Closes #8426, 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)

A source reads a file rather than an upstream port, so it is configured from a
file the runner writes rather than from a table, and the variants follow the
configuration: a CSV with a header and without, an encoding per charset.

Two reports come with it. One prints which operators run and which are
withheld and why, so the state of the set is a command rather than a claim.
The other parses every generated script and fails on one that a hostile column
name would break, which is a whole class of defect a comparison cannot see.

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.

kz930 and others added 2 commits September 2, 2026 17:17
…ady are

It routes a source to the source runner and asks the escaping check for its
findings, and the coverage report in turn reads the operators it discovered.
Split across two changes those references point both ways, and neither
compiles until both land whatever order they land in.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The spec printed a tier per operator, a per-kind tally, and every withheld run
with its reason. That is a progress report: it read the same dispositions the
run itself reads and asserted nothing about them, so it could only ever say what
the run had already decided.

What it did assert stays: a small set of operators that must not become
unrunnable. They take the shared table as it is and nothing about them is hard to
run, so one turning up flagged means something upstream of the disposition broke.

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

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8364      +/-   ##
============================================
- Coverage     94.11%   94.10%   -0.02%     
- Complexity     4811     4858      +47     
============================================
  Files          1197     1203       +6     
  Lines         48813    48967     +154     
  Branches       5906     5930      +24     
============================================
+ Hits          45939    46079     +140     
+ Misses         1420     1419       -1     
- Partials       1454     1469      +15     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø)
agent-service 99.32% <ø> (ø) Carriedforward from d2e44e4
amber 90.08% <ø> (ø) Carriedforward from d2e44e4
computing-unit-managing-service 73.67% <ø> (ø)
config-service 87.12% <ø> (+0.12%) ⬆️
file-service 87.91% <ø> (ø)
frontend 96.86% <ø> (ø) Carriedforward from d2e44e4
notebook-migration-service 83.57% <ø> (+4.26%) ⬆️
pyamber 98.47% <ø> (ø) Carriedforward from d2e44e4
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from d2e44e4

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

Two comments here restated what the code beside them shows: the three tiers a
source dispatches through, which `canRun` and `flagReason` already spell out,
and this spec's own repetition of the runner's dispatch, which belongs to the
runner.

What replaces them says what each file is for, once.

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

@carloea2 carloea2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source verification and coverage checks look good.

@kz930
kz930 marked this pull request as draft September 4, 2026 17:12
kz930 and others added 2 commits September 4, 2026 13:22
The translator names a variable per output PORT, not per reader, so a workflow
that draws two charts from one upstream hands both the same name. An operator
that drops rows into it, or assigns a column through it, changes what the other
branch reads. Ten operators did, and every one of them passed: a fixture with a
single branch has nothing to notice it with.

`StandaloneInputCheck` reads the generated code instead of waiting for a run to
disagree, and reports all three spellings: the assignment, the `inplace=True`,
and the rebinding that looks local but renames the shared variable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`in1df.loc[:, "c"] = v` writes to the input exactly as `in1df["c"] = v` does,
and the pattern read only the plain subscript, so an operator would have passed
for choosing the accessor.

What it still cannot see is a mutation through another name: bind the frame to
something else first and the writes below are invisible to anything reading the
code. The scaladoc says so rather than leaving the check looking airtight.

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

@carloea2 carloea2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new check catches direct changes to input frames. Looks good.

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.

Run the source operators, and report what is covered

3 participants