Skip to content

feat: read the result of any output port of an operator - #9054

Open
kz930 wants to merge 2 commits into
apache:mainfrom
kz930:feat/9053-read-any-output-port
Open

kz930 wants to merge 2 commits into
apache:mainfrom
kz930:feat/9053-read-any-output-port

Conversation

@kz930

@kz930 kz930 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Every external output port of an operator that keeps its result is stored, but results were only ever read from the first port, so a Split's second output or an If's True branch could not be shown.

A pagination request can now name the output port to read, with the first port as the default, and the reply carries the port it read and that port's row count. The live result updates keep following the first port, so a client paging another port takes the count from the reply. The result store's count per operator now comes from the first port too: it was built from every stored port, so a terminal Split's second output could replace its first output's count, and the result panel paged by the wrong number.

On the frontend, the result service gives each extra port its own pagination service, which names its port in its requests, keeps no page cache and takes its row count from each reply. Every port's service takes its schema only from replies to its own requests, since all replies share one stream. The result table and its row detail can show any port; nothing picks a port other than the first yet, which comes with the pinned windows in #9052.

Any related issues, documentation, discussions?

Closes #9050. Closes #9053. Part of #9052.

How was this PR tested?

New cases in ExecutionResultServiceSpec check that a request for each of an operator's two ports returns that port's rows and count, and that an operator with two stored ports is reported by its first port's count; the latter fails on main. PaginatedResultEventSpec checks the reply's new fields and that it serializes to exactly what the frontend declares. New cases in the result service's and the result table's specs cover the per-port services and requests. The backend specs above and the affected frontend specs pass.

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

Generated-by: Claude Code (Claude Opus 5.5)

🤖 Generated with Claude Code

@kz930

kz930 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@carloea2 could you take a look at this when you have a chance? Thanks!

Every external output port of an operator that keeps its result is
stored, but results were only ever read from the first port, so a
Split's second output or an If's True branch could not be shown.

A pagination request can now name the output port to read, with the
first port as the default, and the reply carries the port it read and
that port's row count. The live result updates keep following the
first port, so a client paging another port takes the count from the
reply. The result store's count per operator now comes from the first
port too; it was built from every stored port, so a terminal Split's
second output could replace its first output's count.

On the frontend, the result service gives each extra port its own
pagination service, which names its port in its requests, keeps no
page cache and takes its row count from each reply, and every port's
service takes its schema only from replies to its own requests. The
result table and its row detail can show any port.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kz930
kz930 force-pushed the feat/9053-read-any-output-port branch from de65175 to 8846756 Compare October 20, 2026 23:10
@kz930 kz930 changed the title feat(backend): read the result of any output port of an operator feat: read the result of any output port of an operator Oct 20, 2026

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

110 frontend tests pass locally. Two added port-switching regressions fail at this head.

Switching the table to another output port left the previous port's
rows on screen when the new port was empty, with the loading state
never ending, and a late reply for the previous port could replace the
new port's rows. A port change now starts from an empty table, a reply
for an operator or port no longer shown is dropped, and an empty page
ends the loading.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kz930

kz930 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, both reproduced. Fixed in 13c1294: a port change now starts from an empty table and an empty reply ends the loading, and a reply is dropped if the operator or port it was requested for is no longer shown; the two cases are added as regression tests in result-table-frame.component.spec.ts.

@github-actions github-actions Bot added feature engine frontend Changes related to the frontend GUI labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @aglinxinyuan, @yangzhang75, @Ma77Ball
    You can notify them by mentioning @aglinxinyuan, @yangzhang75, @Ma77Ball in a comment.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.33%. Comparing base (22b82a2) to head (13c1294).

Files with missing lines Patch % Lines
...result-table-frame/result-table-frame.component.ts 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9054      +/-   ##
============================================
- Coverage     92.33%   92.33%   -0.01%     
+ Complexity     5045     5044       -1     
============================================
  Files          1261     1261              
  Lines         54120    54141      +21     
  Branches       6768     6779      +11     
============================================
+ Hits          49974    49993      +19     
  Misses         2486     2486              
- Partials       1660     1662       +2     
Flag Coverage Δ *Carryforward flag
access-control-service 77.38% <ø> (ø) Carriedforward from 22b82a2
agent-service 99.16% <ø> (ø) Carriedforward from 22b82a2
amber 87.31% <ø> (-0.01%) ⬇️ Carriedforward from 22b82a2
computing-unit-managing-service 60.48% <ø> (ø) Carriedforward from 22b82a2
config-service 87.37% <ø> (ø) Carriedforward from 22b82a2
file-service 82.00% <ø> (ø) Carriedforward from 22b82a2
frontend 96.55% <97.43%> (-0.01%) ⬇️
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 22b82a2
pyamber 98.57% <ø> (ø) Carriedforward from 22b82a2
workflow-compiling-service 72.43% <ø> (ø) Carriedforward from 22b82a2

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine feature frontend Changes related to the frontend GUI

Projects

None yet

3 participants