experimental/air: lazily page older runs in air list#5811
Merged
Conversation
The interactive table only ever held the newest `--limit` (20) runs, so its ←/→ paging scrolled the loaded window and could never reach older runs. Replace the one-shot listAirRuns with a stateful runFetcher that pages runs/list on demand and buffers a page's leftover runs, so the table fetches the next batch (and its MLflow links) as the cursor nears the end. One-shot JSON and piped/`--limit` output is unchanged. Also tighten a few comments that had drifted after MLflow enrichment was added. Co-authored-by: Isaac
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
Integration test reportCommit: ff6a3b7
8 interesting tests: 4 flaky, 4 SKIP
Top 29 slowest tests (at least 2 minutes):
|
maggiewang-db
approved these changes
Jul 2, 2026
| if url := m.rows[m.cursor].MLflowURL; url != "" && url != "-" { | ||
| return m, openURL(url) | ||
| } | ||
| if url := m.rows[m.cursor].MLflowURL; url != "" && url != "-" { |
There was a problem hiding this comment.
This drops the previous len(m.rows) > 0 guard, so m.rows[m.cursor] will panic if the model is ever created/updated with zero rows.
Is it intended?
Author
There was a problem hiding this comment.
good point, this path is not currently reachable from the production path through renderListText (since it early returns on empty), but added back the guard since it is reachable via direct model construction (through tests etc)
| m.offset = m.clampedOffset() | ||
| return m, m.maybeFetch() | ||
|
|
||
| case moreRowsMsg: |
There was a problem hiding this comment.
The lazy-paging logic added here has no model-level test — the list_tui_test.go cases all pass a nil fetcher. A test that drives Update with a moreRowsMsg (asserting rows are appended, loading is cleared, and the error branch sets loadErr) would lock in this behavior.
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Restore the len(m.rows) > 0 guard on the enter handler that the paging change dropped, so opening MLflow can't panic on m.rows[m.cursor] when the model holds zero rows (e.g. cursor at -1 after end/G). Make fetchCmd/maybeFetch value receivers returning the updated model, matching Update/View. They were the only pointer-receiver methods on listModel, which tripped the recvcheck linter (mixed receiver types). Add model-level tests for the moreRowsMsg handler: rows are appended and loading cleared on a batch, loadErr is set on the error branch, and an empty page keeps paging until the fetcher is exhausted. Co-authored-by: Isaac
riddhibhagwat-db
force-pushed
the
air-list-pagination
branch
from
1010952 to
ff6a3b7
Compare
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
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.
The interactive
air listtable only ever held the newest--limit(20) runs, so its ←/→ paging just scrolled that fixed window — older runs (e.g. anything before a recent date) were never fetched from the server and could never be reached.Fix
Replace the one-shot
listAirRunswith a statefulrunFetcherthat pages Jobsruns/liston demand:next()calls resume where the last stopped.list_tui.go) holds the fetcher and callsnext(listPageRows)in the background as the cursor nears the end of the loaded rows (maybeFetch→fetchCmd→moreRowsMsg). New rows and their MLflow links are appended and column widths recomputed; only one fetch runs at a time.row N/Mplus(loading…)/(load failed).maxListScan(2000) remains the safety ceiling on total runs scanned.One-shot output paths (JSON, piped, and explicit
--limit) are unchanged: they callfetcher.next(limit)once, so acceptance output is identical.Tests
list/model tests to the newrunFetcher/newListModelsignatures.TestRunFetcherResumesAcrossCalls: anext()that stops mid-page buffers the rest, hands it back on the next call, then reports exhaustion — without re-fetching.air list/air getacceptance tests unchanged and green; build + vet clean.This pull request and its description were written by Isaac.