Add endless-scrolling TUI table for interactive list commands#4729
Open
simonfaltum wants to merge 6 commits intomainfrom
Open
Add endless-scrolling TUI table for interactive list commands#4729simonfaltum wants to merge 6 commits intomainfrom
simonfaltum wants to merge 6 commits intomainfrom
Conversation
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: dbd2241
16 interesting tests: 7 SKIP, 6 RECOVERED, 3 flaky
Top 20 slowest tests (at least 2 minutes):
|
simonfaltum
force-pushed
the
simonfaltum/list-tui-paginated
branch
from
53b85d2 to
64f2b98
Compare
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
force-pushed
the
simonfaltum/list-tui-paginated
branch
from
64f2b98 to
ef8c413
Compare
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Co-authored-by: Isaac
simonfaltum
force-pushed
the
simonfaltum/list-tui-paginated
branch
from
1bf0772 to
e672906
Compare
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
requested review from
a team,
andrewnester,
anton-107,
denik,
lennartkats-db,
pietern and
shreyas-goenka
as code owners
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
shreyas-goenka
left a comment
shreyas-goenka
left a comment
There was a problem hiding this comment.
Note: This review was posted by Claude (AI assistant). Shreyas will do a separate, more thorough review pass.
Priority: HIGH — Performance and design concerns in TUI table infrastructure
MAJOR: O(n) re-render on every keystroke
The search/filter functionality appears to re-render the entire table on each keystroke. For large result sets, this will cause noticeable lag. Consider debouncing the search input or using incremental filtering.
MAJOR: Partially-consumed iterator after search
When a user searches/filters, the iterator may have already been partially consumed. If the search is cleared, previously consumed items that didn't match won't be re-fetched. This could lead to confusing behavior where clearing a search shows fewer results than expected.
MEDIUM: Entire feature unreachable until follow-up PRs
This is 1535 lines of new code that is unreachable until the override PRs (#4731, #4732) land. Consider whether the infrastructure could be reviewed more effectively alongside at least one consumer.
What looks good
- Clean separation of concerns between model, view, and update
- Good use of Bubble Tea framework patterns
- Table column configuration is flexible and well-designed
simonfaltum
force-pushed
the
simonfaltum/list-tui-paginated
branch
from
8fa51c9 to
d7a7104
Compare
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Search input now triggers server-side filtering automatically after the user stops typing for 200ms, instead of waiting for Enter. This prevents redundant API calls on each keystroke while keeping the text input responsive. Enter still executes search immediately, bypassing the debounce. Uses Bubble Tea's tick-based message pattern with a sequence counter to discard stale debounce ticks when the user types additional characters before the delay expires.
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Fix four issues in the paginated TUI: 1. Entering search mode now sets loading=true to prevent maybeFetch from starting new fetches against the shared iterator while in search mode. In-flight fetches are discarded via the generation check. 2. executeSearch sets loading=true (was false) to prevent overlapping fetch commands when a quick scroll triggers maybeFetch before the first search fetch returns. 3. Pressing esc to close search now restores savedRows, savedIter, and savedExhaust (same as clearing the query via enter with empty input). 4. RenderIterator now checks the final model for application-level errors via the new FinalModel interface, since tea.Program.Run() only returns framework errors.
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
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.
Why
Large list commands are hard to use in a terminal today. They print the full result set before users can do anything, which is slow for big lists and makes it hard to browse results. Interactive terminals should get a faster, more navigable experience without changing scripted output.
Changes
Before: list commands drained the full paginated iterator and rendered everything through text templates or JSON.
Now: when the CLI is running interactively (TTY stdin + stdout + stderr, color enabled), list commands with a registered table configuration open a scrollable TUI table that loads more rows on demand. Piped output and
--output jsonkeep the existing behavior. Commands without an explicit table config continue to use template rendering.Implementation:
tableview.gointocommon.gotableviewconfig, registry, and wrapper typesexperimental/aitoolsto the shared static table rendererTest plan
go test ./libs/tableview/... ./libs/cmdio/... ./cmd/root/... ./experimental/aitools/cmd/...--output json: verify existing output is unchangedmake checkspassesmake lintfullpasses (0 issues)