Skip to content

[web-console] Implement search bar for logs tab#6394

Open
Karakatiza666 wants to merge 1 commit into
mainfrom
logs-search
Open

[web-console] Implement search bar for logs tab#6394
Karakatiza666 wants to merge 1 commit into
mainfrom
logs-search

Conversation

@Karakatiza666
Copy link
Copy Markdown
Contributor

@Karakatiza666 Karakatiza666 commented Jun 4, 2026

Unify logs implementation across pipeline page, profiler page and the standalone profiler app

Testing: manual, added unit test

Screencast.from.2026-06-04.21-16-25.webm

Unify logs implementation across pipeline page, profiler page and the standalone profiler app

Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
@Karakatiza666 Karakatiza666 requested a review from mihaibudiu June 4, 2026 15:56
@mihaibudiu
Copy link
Copy Markdown
Contributor

No screenshots?

@@ -0,0 +1,244 @@
<script lang="ts">
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.

Isn't it customary to have a file-level comment explaining what is in the file?

issueCategoryFilter: string
/** Links the metrics node title back to (searches for) the node in the diagram. */
onSearchNode?: (query: string) => void
/** Focus the analysis-panel search input. Invoked from inside a tab (e.g. the log list
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.

You are documenting an API, but the description says how the API is used by an implementation. Wouldn't this comment belong in the implementation?

let lookupQuery = $state('')
// Bound to the shared analysis-panel lookup <input>. Tabs (e.g. the log list) can request
// focus via the `onSearchShortcut` callback threaded through `analysisTabProps`.
let lookupInputEl: HTMLInputElement | undefined = $state()
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.

then why not name it analysisPanelElement?

onSearchShortcut
}: {
logs: { rows: string[]; totalSkippedBytes: number; firstRowIndex: number }
/** Current search; the host advances `occurrenceIndex` (wraps modulo match count) and
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.

where is occurenceIndex?

observeContentElement: (e) => e.firstElementChild!
})
let virtualizer: VirtualizerHandle = $state()!
// Streaming-log rows from the manager already carry trailing newlines, so the default
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.

this comment should be on the join statement, and it can be much shorter

* highlight anything" — keeps callers free of empty-query / bad-regex branching. */
export type LineMatcher = ((line: string) => boolean) | null

export function compileSearchPattern(pattern: SearchPattern): LineMatcher {
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.

could all this search infra be reused for other components like the profiler?
Then maybe it should not be called "logSearch"

export function findMatchOffsets(
line: string,
pattern: SearchPattern
): Array<readonly [number, number]> {
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.

how about defining a type instead of using a list of numbers?

}
// Walk text nodes once, recording each one's [start, end) span in the element's flattened
// text. Looking up a Range endpoint is then a linear scan over this list — cheap, given
// we only do it for a handful of matches on a single line.
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.

cheap for a fixed log content, but the log content may refresh too

// we only do it for a handful of matches on a single line.
const spans: Array<{ node: Text; start: number; end: number }> = []
const walker = document.createTreeWalker(el, NodeFilter.SHOW_TEXT)
let cum = 0
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.

call this "total"

* No-op (silently clears) on browsers that don't expose `CSS.highlights` or the global
* `Highlight` constructor (older Safari/Firefox).
*/
export function applySearchHighlight(
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.

I hope this is robust, some tests for it could be useful

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants