feat: add searchable drilldown views#88
Open
ya-nsh wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThis PR introduces drill-down filtering capabilities to the tabbed dashboard and terminal renderers. It adds a new DrilldownFilterState type with filtering and sorting utilities, extends the TabbedDashboardOptions API with a promptInput callback, and integrates filter state into session and project view rendering. Test coverage includes new filtering and search scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dashboard as TabbedDashboard
participant Filter as Filter Parser
participant SessionView as Session/Project View
participant FilterUtil as Filter Utilities
User->>Dashboard: Press '/' or 'f' to search/filter
Dashboard->>Dashboard: promptInput called
User->>Dashboard: Enter filter tokens (e.g., provider:foo model:bar)
Dashboard->>Filter: parseFilterTokens(input)
Filter->>Filter: normalizeFilterState(tokens)
Filter-->>Dashboard: Updated DrilldownFilterState
Dashboard->>SessionView: renderActiveView(..., drilldownFilter)
SessionView->>FilterUtil: getFilteredSessions(output, filterState)
FilterUtil->>FilterUtil: Filter by provider, model, query
FilterUtil->>FilterUtil: Sort sessions by cost/duration/tokens
FilterUtil-->>SessionView: Filtered & sorted results + counts
SessionView->>SessionView: Display filter summary
SessionView->>SessionView: Render filtered sessions
SessionView-->>Dashboard: Rendered view
Dashboard-->>User: Display filtered results with active filter indicator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/tabbed-dashboard.ts (1)
179-183:⚠️ Potential issue | 🟠 MajorAccount for wrapped footer lines when computing the viewport.
Line 181 still budgets exactly one footer row, but Lines 218-226 now render a footer that is much wider than the supported 40-column minimum from Line 171. On narrow terminals it wraps into multiple rows, so the viewport is oversized and the bottom of the screen gets clipped/overwritten.
Suggested fix
+function getWrappedLineCount(text: string, width: number): number { + const plain = text.replace(/\x1b\[[0-9;]*m/g, ''); + return Math.max(1, Math.ceil(plain.length / width)); +} + function getViewportHeight(state: TabbedState, width: number, rows: number): number { const headerLines = renderTabBar(state.timeRange, state.metricTab, width, state.noColor).split('\n').length + 2; - const footerLines = 1; + const footerLines = getWrappedLineCount(renderFooter(state), width); return Math.max(4, rows - headerLines - footerLines - 1); }Also applies to: 217-233
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/tabbed-dashboard.ts` around lines 179 - 183, getViewportHeight currently assumes footerLines = 1 which breaks when the footer (rendered later) wraps on narrow terminals; change getViewportHeight to compute footerLines by rendering the same footer string with the provided width and counting its newline-wrapped lines (e.g., call the footer rendering used at lines 218-226—render the footer with the given width and use .split('\n').length) and subtract that from rows instead of the hardcoded 1 so the viewport height correctly accounts for wrapped footer lines.
🧹 Nitpick comments (3)
packages/renderers/src/terminal/tab-views/searchable-drilldown.ts (2)
49-66: Clarify the dual-purpose ofactiveflag vs field presence.The function returns
trueif eitherfilterState.activeis true OR any filter field is non-empty. This means settingactive: truewith all empty filters still indicates "active filters" to the UI.This design seems intentional for tracking whether the filter UI is engaged, but it may be worth adding a brief doc comment to clarify this behavior for future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderers/src/terminal/tab-views/searchable-drilldown.ts` around lines 49 - 66, Add a short doc comment above the hasActiveDrilldownFilters function explaining that the boolean return is true when either the explicit flag filterState.active is set (used to indicate the UI's filter panel is engaged) OR when any of the filter text fields (query, provider, project, model, sort) contain non-empty values; mention that filterState.active can be true even if all fields are empty and that both conditions are considered "active filters" by the UI. This clarifies intent for future maintainers when they read hasActiveDrilldownFilters and the related DrilldownFilterState fields.
29-31: Unused generic parameters insortByText.The parameters
aandbare declared but never used in the function body. This makes the generic signature misleading.♻️ Proposed simplification
-function sortByText<T>(a: T, b: T, left: string, right: string): number { +function sortByText(left: string, right: string): number { return left.localeCompare(right); }Then update call sites to remove the first two arguments:
-|| sortByText(left, right, left.label, right.label); +|| sortByText(left.label, right.label);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderers/src/terminal/tab-views/searchable-drilldown.ts` around lines 29 - 31, The function sortByText currently declares an unused generic and unused parameters a and b; remove the generic <T> and the unused parameters so the signature becomes sortByText(left: string, right: string): number, update all call sites that pass four args to instead pass only the two string arguments (left and right), and ensure callers that relied on the previous signature are adjusted to supply the correct arguments to sortByText.packages/renderers/src/terminal/tab-views/session-view.ts (1)
83-120: Consider adding a display limit for filtered sessions.The filtered sessions are rendered without an explicit limit (line 87 assigns all
filteredtosessions). With a large dataset and broad filters, this could produce very long terminal output.If this is intentional to show all matches in filtered mode, this is fine. Otherwise, consider adding a configurable or fixed limit (e.g., top 10 or 20).
♻️ Optional: Add display limit
- const sessions = filtered; + const sessions = filtered.slice(0, 20); // Limit display to top 20🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderers/src/terminal/tab-views/session-view.ts` around lines 83 - 120, The code currently assigns all filtered entries to sessions and renders them unbounded; change this to enforce a display cap (e.g., const maxDisplay = 20 or a passed-in config) and set sessions = filtered.slice(0, maxDisplay); inside the rendering block, keep using the same loop over sessions but ensure color indexing uses the loop index (PROJECT_COLORS[index % PROJECT_COLORS.length]) as already written, and after the loop add a single summary/truncation line (e.g., " showing first X of Y matches") when filtered.length > maxDisplay so users know output was truncated; update any references to sessions, clampLabel, formatTokens, formatCost, and formatDuration accordingly to work with the sliced array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/tabbed-dashboard.ts`:
- Around line 383-412: promptForInput currently allows late/old prompt
resolutions to mutate state or trigger repaints; fix by stamping each prompt
with a unique token/counter when started and, after any await (including when
options.promptInput is used) and before mutating state or calling
repaint/setting state.drilldownFilter, verify that the token still matches the
active prompt and that shouldClose is false. Apply the same guard to the other
prompt block handling standard input (the onData resolution path) and to the
similar prompt handler referenced in the comment so stale results are discarded
instead of applied.
- Around line 97-141: parseFilterTokens currently splits the input on raw
whitespace (trimmed.split(/\s+/)), which breaks values containing spaces (e.g.,
project=/Users/me/My App); replace the naive splitting with a robust key=value
extractor that matches key=value pairs allowing quoted or unquoted values (for
example using a global regex that captures key and value like
key=(quoted|string-without-spaces)), unquote values when present, then assign
into the next DrilldownFilterState (same keys used: query, provider, project,
model, sort) and finally call normalizeFilterState(next) as before; update
parseFilterTokens to use this tokenizer/regex approach instead of trimmed.split
to preserve spaces in values.
---
Outside diff comments:
In `@packages/cli/src/tabbed-dashboard.ts`:
- Around line 179-183: getViewportHeight currently assumes footerLines = 1 which
breaks when the footer (rendered later) wraps on narrow terminals; change
getViewportHeight to compute footerLines by rendering the same footer string
with the provided width and counting its newline-wrapped lines (e.g., call the
footer rendering used at lines 218-226—render the footer with the given width
and use .split('\n').length) and subtract that from rows instead of the
hardcoded 1 so the viewport height correctly accounts for wrapped footer lines.
---
Nitpick comments:
In `@packages/renderers/src/terminal/tab-views/searchable-drilldown.ts`:
- Around line 49-66: Add a short doc comment above the hasActiveDrilldownFilters
function explaining that the boolean return is true when either the explicit
flag filterState.active is set (used to indicate the UI's filter panel is
engaged) OR when any of the filter text fields (query, provider, project, model,
sort) contain non-empty values; mention that filterState.active can be true even
if all fields are empty and that both conditions are considered "active filters"
by the UI. This clarifies intent for future maintainers when they read
hasActiveDrilldownFilters and the related DrilldownFilterState fields.
- Around line 29-31: The function sortByText currently declares an unused
generic and unused parameters a and b; remove the generic <T> and the unused
parameters so the signature becomes sortByText(left: string, right: string):
number, update all call sites that pass four args to instead pass only the two
string arguments (left and right), and ensure callers that relied on the
previous signature are adjusted to supply the correct arguments to sortByText.
In `@packages/renderers/src/terminal/tab-views/session-view.ts`:
- Around line 83-120: The code currently assigns all filtered entries to
sessions and renders them unbounded; change this to enforce a display cap (e.g.,
const maxDisplay = 20 or a passed-in config) and set sessions =
filtered.slice(0, maxDisplay); inside the rendering block, keep using the same
loop over sessions but ensure color indexing uses the loop index
(PROJECT_COLORS[index % PROJECT_COLORS.length]) as already written, and after
the loop add a single summary/truncation line (e.g., " showing first X of Y
matches") when filtered.length > maxDisplay so users know output was truncated;
update any references to sessions, clampLabel, formatTokens, formatCost, and
formatDuration accordingly to work with the sliced array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10acac21-a800-4169-8d76-462142a4ddb7
📒 Files selected for processing (9)
packages/cli/src/tabbed-dashboard.test.tspackages/cli/src/tabbed-dashboard.tspackages/renderers/src/index.tspackages/renderers/src/terminal/index.tspackages/renderers/src/terminal/tab-views/cwd-view.tspackages/renderers/src/terminal/tab-views/index.tspackages/renderers/src/terminal/tab-views/searchable-drilldown.tspackages/renderers/src/terminal/tab-views/session-view.tspackages/renderers/src/terminal/tab-views/tab-views.test.ts
Comment on lines
+97
to
+141
| function parseFilterTokens( | ||
| input: string, | ||
| current: DrilldownFilterState, | ||
| ): DrilldownFilterState { | ||
| const trimmed = input.trim(); | ||
| if (!trimmed) { | ||
| return current; | ||
| } | ||
|
|
||
| if (trimmed.toLowerCase() === 'clear') { | ||
| return { ...EMPTY_DRILLDOWN_FILTER_STATE }; | ||
| } | ||
|
|
||
| const next: DrilldownFilterState = { ...current }; | ||
| for (const token of trimmed.split(/\s+/)) { | ||
| const separator = token.indexOf('='); | ||
| if (separator <= 0) { | ||
| continue; | ||
| } | ||
|
|
||
| const key = token.slice(0, separator).trim().toLowerCase(); | ||
| const value = token.slice(separator + 1).trim(); | ||
| switch (key) { | ||
| case 'query': | ||
| next.query = value; | ||
| break; | ||
| case 'provider': | ||
| next.provider = value; | ||
| break; | ||
| case 'project': | ||
| next.project = value; | ||
| break; | ||
| case 'model': | ||
| next.model = value; | ||
| break; | ||
| case 'sort': | ||
| next.sort = value.toLowerCase(); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return normalizeFilterState(next); | ||
| } |
There was a problem hiding this comment.
Don't split structured filters on raw whitespace.
Line 111 truncates values at the first space before the key=value parsing runs. That breaks legitimate inputs like project=/Users/me/My App, so the new cwd/project drilldown becomes unfilterable for common project paths with spaces.
Suggested fix
- for (const token of trimmed.split(/\s+/)) {
+ for (const token of trimmed.split(/\s+(?=(?:query|provider|project|model|sort)=)/i)) {
const separator = token.indexOf('=');
if (separator <= 0) {
continue;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function parseFilterTokens( | |
| input: string, | |
| current: DrilldownFilterState, | |
| ): DrilldownFilterState { | |
| const trimmed = input.trim(); | |
| if (!trimmed) { | |
| return current; | |
| } | |
| if (trimmed.toLowerCase() === 'clear') { | |
| return { ...EMPTY_DRILLDOWN_FILTER_STATE }; | |
| } | |
| const next: DrilldownFilterState = { ...current }; | |
| for (const token of trimmed.split(/\s+/)) { | |
| const separator = token.indexOf('='); | |
| if (separator <= 0) { | |
| continue; | |
| } | |
| const key = token.slice(0, separator).trim().toLowerCase(); | |
| const value = token.slice(separator + 1).trim(); | |
| switch (key) { | |
| case 'query': | |
| next.query = value; | |
| break; | |
| case 'provider': | |
| next.provider = value; | |
| break; | |
| case 'project': | |
| next.project = value; | |
| break; | |
| case 'model': | |
| next.model = value; | |
| break; | |
| case 'sort': | |
| next.sort = value.toLowerCase(); | |
| break; | |
| default: | |
| break; | |
| } | |
| } | |
| return normalizeFilterState(next); | |
| } | |
| function parseFilterTokens( | |
| input: string, | |
| current: DrilldownFilterState, | |
| ): DrilldownFilterState { | |
| const trimmed = input.trim(); | |
| if (!trimmed) { | |
| return current; | |
| } | |
| if (trimmed.toLowerCase() === 'clear') { | |
| return { ...EMPTY_DRILLDOWN_FILTER_STATE }; | |
| } | |
| const next: DrilldownFilterState = { ...current }; | |
| for (const token of trimmed.split(/\s+(?=(?:query|provider|project|model|sort)=)/i)) { | |
| const separator = token.indexOf('='); | |
| if (separator <= 0) { | |
| continue; | |
| } | |
| const key = token.slice(0, separator).trim().toLowerCase(); | |
| const value = token.slice(separator + 1).trim(); | |
| switch (key) { | |
| case 'query': | |
| next.query = value; | |
| break; | |
| case 'provider': | |
| next.provider = value; | |
| break; | |
| case 'project': | |
| next.project = value; | |
| break; | |
| case 'model': | |
| next.model = value; | |
| break; | |
| case 'sort': | |
| next.sort = value.toLowerCase(); | |
| break; | |
| default: | |
| break; | |
| } | |
| } | |
| return normalizeFilterState(next); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/tabbed-dashboard.ts` around lines 97 - 141,
parseFilterTokens currently splits the input on raw whitespace
(trimmed.split(/\s+/)), which breaks values containing spaces (e.g.,
project=/Users/me/My App); replace the naive splitting with a robust key=value
extractor that matches key=value pairs allowing quoted or unquoted values (for
example using a global regex that captures key and value like
key=(quoted|string-without-spaces)), unquote values when present, then assign
into the next DrilldownFilterState (same keys used: query, provider, project,
model, sort) and finally call normalizeFilterState(next) as before; update
parseFilterTokens to use this tokenizer/regex approach instead of trimmed.split
to preserve spaces in values.
Comment on lines
+383
to
+412
| const promptForInput = async ( | ||
| kind: DashboardPromptKind, | ||
| label: string, | ||
| ): Promise<string | null> => { | ||
| if (options.promptInput) { | ||
| return options.promptInput(kind, label); | ||
| } | ||
|
|
||
| return await new Promise<string | null>((resolve) => { | ||
| if (process.stdin.isTTY) { | ||
| process.stdin.setRawMode(false); | ||
| } | ||
| process.stdin.resume(); | ||
| process.stdout.write(`${HOME_CLEAR}${SHOW_CURSOR}${label}`); | ||
|
|
||
| const onData = (chunk: string | Uint8Array): void => { | ||
| process.stdin.off('data', onData); | ||
| if (process.stdin.isTTY) { | ||
| process.stdin.setRawMode(true); | ||
| } | ||
| process.stdin.resume(); | ||
| process.stdout.write(HIDE_CURSOR); | ||
|
|
||
| const text = String(chunk).replace(/\r?\n$/, ''); | ||
| resolve(text); | ||
| }; | ||
|
|
||
| process.stdin.on('data', onData); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Discard stale prompt results before mutating state or repainting.
promptInput is now a public async hook, and the handlers never check shouldClose or prompt ordering after await. A late resolution can still update state.drilldownFilter and repaint after teardown, or an older prompt can overwrite a newer one.
Suggested fix
let currentOutput: TokenleakOutput | null = null;
let activeLoadId = 0;
+ let activePromptId = 0;
let shouldClose = false;
@@
if (key.sequence === '/' && isSearchableTab(state.metricTab)) {
runAsyncAction(async () => {
+ const promptId = ++activePromptId;
const input = await promptForInput('query', 'Search sessions/projects: ');
+ if (shouldClose || promptId !== activePromptId) {
+ return;
+ }
if (input !== null && input.trim()) {
state.drilldownFilter = normalizeFilterState({
...state.drilldownFilter,
query: input,
});
}
rerender();
});
return;
}
@@
if (key.name === 'f' && isSearchableTab(state.metricTab)) {
runAsyncAction(async () => {
+ const promptId = ++activePromptId;
const input = await promptForInput(
'filter',
'Filters (provider=.. project=.. model=.. sort=..): ',
);
+ if (shouldClose || promptId !== activePromptId) {
+ return;
+ }
if (input !== null) {
state.drilldownFilter = parseFilterTokens(input, state.drilldownFilter);
}
rerender();
});
return;
}Also applies to: 453-477
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/tabbed-dashboard.ts` around lines 383 - 412, promptForInput
currently allows late/old prompt resolutions to mutate state or trigger
repaints; fix by stamping each prompt with a unique token/counter when started
and, after any await (including when options.promptInput is used) and before
mutating state or calling repaint/setting state.drilldownFilter, verify that the
token still matches the active prompt and that shouldClose is false. Apply the
same guard to the other prompt block handling standard input (the onData
resolution path) and to the similar prompt handler referenced in the comment so
stale results are discarded instead of applied.
ya-nsh
commented
Mar 15, 2026
ya-nsh
left a comment
ya-nsh
left a comment
Owner
Author
There was a problem hiding this comment.
Overall solid feature. A few issues worth addressing before merge — one regression (session list unbounded) and a couple of semantic/dead-code nits.
| return (value ?? '').trim().toLowerCase(); | ||
| } | ||
|
|
||
| function sortByText<T>(a: T, b: T, left: string, right: string): number { |
Owner
Author
There was a problem hiding this comment.
Dead parameters — a: T and b: T are never used inside the body; only left and right are referenced. Simplify to:
function sortByText(left: string, right: string): number {
return left.localeCompare(right);
}and drop the first two arguments from all call sites.
| normalizeText(filterState.provider) || | ||
| normalizeText(filterState.project) || | ||
| normalizeText(filterState.model) || | ||
| normalizeText(filterState.sort), |
Owner
Author
There was a problem hiding this comment.
Including sort here means hasActiveDrilldownFilters returns true when the user has only changed sort order, which triggers downstream side-effects (attribution view hidden, "Top project" row hidden, footer shows "filter active"). Sort is a presentation preference, not a filter — it never reduces the result set. Consider removing normalizeText(filterState.sort) from this check, or introduce a separate hasActiveFilters / hasActiveSortOrFilter distinction.
| lines.push(` ${bold('Top Sessions', noColor)}`); | ||
|
|
||
| const sessions = drilldown.slice(0, 5); | ||
| const sessions = filtered; |
Owner
Author
There was a problem hiding this comment.
Before this PR, sessions were capped at 5 (drilldown.slice(0, 5)). Now filtered is unbounded — a user with 200 sessions will get a 200-row table on every render. The old cap should be preserved for the unfiltered default case, or a configurable page size added. At minimum, document this intentional behaviour change in the PR description.
| return; | ||
| } | ||
|
|
||
| if (key.name === 'c') { |
Owner
Author
There was a problem hiding this comment.
/ and f are guarded by isSearchableTab, but c (clear) fires on every tab. This is a minor inconsistency: pressing c on the overview tab silently resets filters the user cannot see. Consider mirroring the guard: if (key.name === 'c' && isSearchableTab(state.metricTab)).
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
1 participant
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.
Summary
Testing
Summary by CodeRabbit
Release Notes
New Features
/to search,fto apply filters,cto clear filtersTests