app/vmui: improve stream context modal#1439
Conversation
- warn and highlight logs with identical timestamps - show older logs above and newer logs below the target log - replace load control buttons with infinite scroll - expand stream_context time_window from 1m to 7d while loading context
There was a problem hiding this comment.
3 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/vmui/packages/vmui/src/pages/StreamContext/helpers.ts">
<violation number="1" location="app/vmui/packages/vmui/src/pages/StreamContext/helpers.ts:74">
P2: Removing by `_stream_id` + `_time` drops all same-timestamp logs in a stream, so valid context entries can disappear when timestamps are duplicated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| /** Merges fetched logs and removes the target log. */ | ||
| export const mergeContextLogs = (dir: Direction, setter: Dispatch<SetStateAction<Logs[]>>) => | ||
| (fetched: Logs[], target: Logs) => { | ||
| const filtered = removeLogsByKeys(fetched, target, ["_stream_id", "_time"]); |
There was a problem hiding this comment.
P2: Removing by _stream_id + _time drops all same-timestamp logs in a stream, so valid context entries can disappear when timestamps are duplicated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/vmui/packages/vmui/src/pages/StreamContext/helpers.ts, line 74:
<comment>Removing by `_stream_id` + `_time` drops all same-timestamp logs in a stream, so valid context entries can disappear when timestamps are duplicated.</comment>
<file context>
@@ -0,0 +1,86 @@
+/** Merges fetched logs and removes the target log. */
+export const mergeContextLogs = (dir: Direction, setter: Dispatch<SetStateAction<Logs[]>>) =>
+ (fetched: Logs[], target: Logs) => {
+ const filtered = removeLogsByKeys(fetched, target, ["_stream_id", "_time"]);
+ setter(prev => dir === "after" ? prev.concat(filtered) : filtered.concat(prev));
+ };
</file context>
There was a problem hiding this comment.
The context query anchors on _stream_id and _time, while the original row may contain query-time transformed fields, so matching by the full row or _msg is unreliable. For duplicate timestamps within the same stream we prefer removing all ambiguous anchor candidates instead of risking rendering the target log twice. The duplicate timestamp warning still applies to duplicate timestamps that remain in the loaded context.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Signed-off-by: Yury Moladau <[email protected]>
Describe Your Changes
Log contextmodal: previous logs are shown above the target log, next logs are shown below itLoad morecontrols and the logs-per-load selector with top/bottom infinite scrollstream_context time_window: start from1m, expand exponentially when fewer logs than requested are returned, and stop at7d. Related issue: app/vmui: allow configuringstream_contexttime_windowin Log Context #1397_timevalues, show a warning, and highlight their timestamp values