feat(console): improve observer monitor formatting and readability#1193
feat(console): improve observer monitor formatting and readability#1193Zouuabi wants to merge 1 commit intovolcengine:mainfrom
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
- Parse ASCII table output into proper HTML tables with headers and rows - Add component health indicators with visual status dots (ok/error) - Create auto-refresh toggle for observer monitoring (10s interval) - Render system status as key-value grid instead of raw JSON - Organize components as cards: Queue, VectorDB, VLM, Retrieval, Locks - Add styling for monitor dashboard (cards, tables, health dots) - Use DOM APIs (createElement/textContent) instead of innerHTML to prevent XSS Contributed by the engineering team at Homesage.ai
e6e40a5 to
09669f1
Compare
qin-ctx
left a comment
There was a problem hiding this comment.
Thanks for the monitor UI improvement. I found three blocking issues that need to be addressed before this can merge.
parseAsciiTable()does not handle the models observer output format, so the Language Model card will not render as a proper table.- The frontend assumes a
vlmcomponent key, but the backend returnsmodels, which breaks naming and ordering. - Auto-refresh keeps running after leaving the monitor view and continues overwriting the shared Result pane.
Please address these and I can re-review.
| const statusText = comp.status || ""; | ||
| const tables = statusText.split("\n\n").filter(Boolean); | ||
| for (const tableText of tables) { | ||
| const parsed = parseAsciiTable(tableText); |
There was a problem hiding this comment.
[Bug] (blocking) parseAsciiTable() assumes every comp.status block is a plain tabulate table, but ModelsObserver.get_status_table() prefixes each table with section headers like VLM Models: before the ASCII table. In that case dataLines[0] becomes the section header, splitRow() returns [], and the real header row is rendered as body data. The Language Model card therefore does not render as the formatted table described in this PR. Please strip leading non-table lines or parse section headers separately before calling renderMonitorTable().
| const friendlyNames = { | ||
| queue: "Processing Queue", | ||
| vikingdb: "Vector Database", | ||
| vlm: "Language Model", |
There was a problem hiding this comment.
[Bug] (blocking) This code assumes the backend component key is vlm, but ObserverService.system() actually returns models (queue, vikingdb, models, lock, retrieval). As a result, this card keeps the raw models label and also falls out of the intended order because both friendlyNames and the order array are keyed on vlm. Please align the frontend mapping with the actual response contract.
|
|
||
| elements.observerBtn.addEventListener("click", loadObserver); | ||
|
|
||
| elements.monitorRefreshBtn.addEventListener("click", () => { |
There was a problem hiding this comment.
[Bug] (blocking) Once auto-refresh is enabled, nothing turns it off when the user leaves the Monitor panel or switches to the System Status view. setInterval(loadObserver, 10000) keeps firing in the background and loadObserver() keeps calling setOutput(payload), so the shared Result pane in other panels gets overwritten every 10 seconds. This is a behavior regression introduced by the new feature. Please clear the interval when leaving the Monitor panel and when switching away from observer data.
Summary
Before / After
Before: Raw ASCII tables and JSON dumps in a
<pre>blockAfter: Formatted HTML tables with health indicators, organized in cards
Type of Change
Testing
Files Changed
openviking/console/static/index.html-- added dashboard container and auto-refresh buttonopenviking/console/static/app.js-- ASCII table parser, component card renderer, auto-refresh logicopenviking/console/static/styles.css-- monitor dashboard card and table stylingChecklist
Contributed by the engineering team at Homesage.ai