experimental/air: lazily page older runs in air list#5811
Open
riddhibhagwat-db wants to merge 1 commit into
Open
experimental/air: lazily page older runs in air list#5811riddhibhagwat-db wants to merge 1 commit into
air list#5811riddhibhagwat-db wants to merge 1 commit into
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: 29208e4
23 interesting tests: 13 SKIP, 10 RECOVERED
Top 30 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?
| 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.
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.