FIX: Reduce logs panel noise and make execution messages readable (OR-1457)#59
Open
abhizipstack wants to merge 1 commit intomainfrom
Open
FIX: Reduce logs panel noise and make execution messages readable (OR-1457)#59abhizipstack wants to merge 1 commit intomainfrom
abhizipstack wants to merge 1 commit intomainfrom
Conversation
Layer 1 — silence the worst offender
- Demote UsingCachedObject from InfoLevel to DebugLevel so the
per-query "Using Cached Object of bigquery_connection_object" spam
no longer reaches the UI at default verbosity.
Layer 2A — fix data bugs leaking dev internals into the UI
- ProcessingModel: avoid Processing Model : "" by using cls.__name__
with file_name as a fallback, instead of str(cls) which can be empty
for dynamically generated no-code model classes.
- ExecStatus / Materialization: stop str(Enum) from leaking the dotted
enum names (ExecStatus.Success, Materialization.EPHEMERAL) into user
messages; use .value so the UI shows SUCCESS / OK / TABLE.
- Summary counts: parse_and_fire_reports compared end_status against
ExecStatus.Success/Error but end_status is set to OK/Fail, so
pass/error counters stayed zero. Compare against the correct
end-status values so DONE PASS=N now reflects reality.
Frontend
- Socket handler now captures the level alongside the message; each
log entry is stored as { level, message } instead of a raw string.
- New log-level selector in the bottom logs tab:
All logs / Info & above / Warnings & errors / Errors only
Default is "Info & above" (hides debug noise out of the box).
Choice persists in localStorage under visitran.logsLevel.
- Log rows are tinted by severity (grey/default/amber/red) for quick
scanning when All is selected.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| backend/visitran/events/printer.py | Fixes counter bug: replaces incorrect str(ExecStatus.X) comparisons with ExecStatus.X.value and converts individual if to elif chain — both changes are correct. |
| backend/visitran/events/types.py | Demotes UsingCachedObject from InfoLevel to DebugLevel to suppress per-query cache-hit noise from the default UI view — straightforward one-line change. |
| backend/visitran/visitran.py | Consistently replaces str(ExecStatus.X) with .value across all call sites and fixes ProcessingModel to use cls.__name__ instead of str(cls) — all changes look correct. |
| frontend/src/ide/editor/no-code-model/no-code-model.jsx | Adds log-level filter UI with localStorage persistence and per-level row colouring; introduces an incomplete socket cleanup (missing disconnect() and potential session-ID mismatch) and recreates filter constants on every render. |
Sequence Diagram
sequenceDiagram
participant VT as visitran.py
participant PR as printer.py
participant EV as events/types.py
participant SK as Socket Server
participant FE as no-code-model.jsx
VT->>EV: fire_event(ExecutingModelNode(materialization=.value))
VT->>EV: fire_event(ProcessingModel(cls=cls.__name__ or file_name))
EV-->>SK: emit logs:{sessionId} {level, message}
VT->>PR: BASE_RESULT.append(BaseResult(end_status=ExecStatus.OK.value))
VT->>PR: parse_and_fire_reports()
PR->>PR: end_status == OK → pass_count++
PR->>EV: fire_event(EndSummaryReportCounts(pass=N ...))
EV-->>SK: emit logs:{sessionId} {level:info, message:DONE PASS=N ...}
Note over EV: UsingCachedObject now DebugLevel
SK-->>FE: logs:{sessionId} {data: {level, message}}
FE->>FE: setLogsInfo([...old, { level, message }])
FE->>FE: filteredLogs = filter by LOG_LEVEL_RANK
FE->>FE: render rows tinted by LOG_LEVEL_COLOR[level]
Comments Outside Diff (1)
-
frontend/src/ide/editor/no-code-model/no-code-model.jsx, line 1826-1829 (link)Incomplete socket cleanup — connection leaks on unmount
The cleanup function calls
newSocket.off(...)but never callsnewSocket.disconnect(), so the underlying socket connection stays alive after the component unmounts. This will triggersetLogsInfoon an unmounted component whenever a new log message arrives, causing React's "update on unmounted component" warning and a memory leak.There's also a session-ID mismatch: the listener was registered on
logs:${innerSessionId}(line 1810, from the socket"session_id"event), while the cleanup removeslogs:${outerSessionId}(line 153,Cookies.get("sessionid")). If the socket server assigns a different ID than the cookie, the listener is never removed.If the server-assigned session ID can differ from the cookie value,
innerSessionIdshould be hoisted to a variable in theuseEffectscope so the cleanup can reference the correct channel.Prompt To Fix With AI
This is a comment left during a code review. Path: frontend/src/ide/editor/no-code-model/no-code-model.jsx Line: 1826-1829 Comment: **Incomplete socket cleanup — connection leaks on unmount** The cleanup function calls `newSocket.off(...)` but never calls `newSocket.disconnect()`, so the underlying socket connection stays alive after the component unmounts. This will trigger `setLogsInfo` on an unmounted component whenever a new log message arrives, causing React's "update on unmounted component" warning and a memory leak. There's also a session-ID mismatch: the listener was registered on `logs:${innerSessionId}` (line 1810, from the socket `"session_id"` event), while the cleanup removes `logs:${outerSessionId}` (line 153, `Cookies.get("sessionid")`). If the socket server assigns a different ID than the cookie, the listener is never removed. If the server-assigned session ID can differ from the cookie value, `innerSessionId` should be hoisted to a variable in the `useEffect` scope so the cleanup can reference the correct channel. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: frontend/src/ide/editor/no-code-model/no-code-model.jsx
Line: 1826-1829
Comment:
**Incomplete socket cleanup — connection leaks on unmount**
The cleanup function calls `newSocket.off(...)` but never calls `newSocket.disconnect()`, so the underlying socket connection stays alive after the component unmounts. This will trigger `setLogsInfo` on an unmounted component whenever a new log message arrives, causing React's "update on unmounted component" warning and a memory leak.
There's also a session-ID mismatch: the listener was registered on `logs:${innerSessionId}` (line 1810, from the socket `"session_id"` event), while the cleanup removes `logs:${outerSessionId}` (line 153, `Cookies.get("sessionid")`). If the socket server assigns a different ID than the cookie, the listener is never removed.
```suggestion
return () => {
newSocket.off(`logs:${sessionId}`);
newSocket.disconnect();
};
```
If the server-assigned session ID can differ from the cookie value, `innerSessionId` should be hoisted to a variable in the `useEffect` scope so the cleanup can reference the correct channel.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: frontend/src/ide/editor/no-code-model/no-code-model.jsx
Line: 372-383
Comment:
**Constants and derived state recreated on every render**
`LOG_LEVEL_RANK`, `LOG_LEVEL_COLOR`, and `filteredLogs` are all recreated on every render. As `logsInfo` grows (common in long runs), the filter pass over the full array on every unrelated state change will become noticeable. Move the two lookup objects outside the component and wrap `filteredLogs` in `useMemo`:
```js
// outside the component
const LOG_LEVEL_RANK = { debug: 0, info: 1, warn: 2, warning: 2, error: 3 };
const LOG_LEVEL_COLOR = { debug: "#8c8c8c", info: undefined, warn: "#d48806", warning: "#d48806", error: "#cf1322" };
// inside the component
const filteredLogs = useMemo(
() => logsInfo.filter(
(entry) => (LOG_LEVEL_RANK[entry.level] ?? 1) >= (LOG_LEVEL_RANK[logsLevel] ?? 1)
),
[logsInfo, logsLevel]
);
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: reduce logs-panel noise and make me..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+372
to
+383
| const LOG_LEVEL_RANK = { debug: 0, info: 1, warn: 2, warning: 2, error: 3 }; | ||
| const LOG_LEVEL_COLOR = { | ||
| debug: "#8c8c8c", | ||
| info: undefined, | ||
| warn: "#d48806", | ||
| warning: "#d48806", | ||
| error: "#cf1322", | ||
| }; | ||
| const filteredLogs = logsInfo.filter( | ||
| (entry) => | ||
| (LOG_LEVEL_RANK[entry.level] ?? 1) >= (LOG_LEVEL_RANK[logsLevel] ?? 1) | ||
| ); |
There was a problem hiding this comment.
LOG_LEVEL_RANK, LOG_LEVEL_COLOR, and filteredLogs are all recreated on every render. As logsInfo grows (common in long runs), the filter pass over the full array on every unrelated state change will become noticeable. Move the two lookup objects outside the component and wrap filteredLogs in useMemo:
// outside the component
const LOG_LEVEL_RANK = { debug: 0, info: 1, warn: 2, warning: 2, error: 3 };
const LOG_LEVEL_COLOR = { debug: "#8c8c8c", info: undefined, warn: "#d48806", warning: "#d48806", error: "#cf1322" };
// inside the component
const filteredLogs = useMemo(
() => logsInfo.filter(
(entry) => (LOG_LEVEL_RANK[entry.level] ?? 1) >= (LOG_LEVEL_RANK[logsLevel] ?? 1)
),
[logsInfo, logsLevel]
);Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/ide/editor/no-code-model/no-code-model.jsx
Line: 372-383
Comment:
**Constants and derived state recreated on every render**
`LOG_LEVEL_RANK`, `LOG_LEVEL_COLOR`, and `filteredLogs` are all recreated on every render. As `logsInfo` grows (common in long runs), the filter pass over the full array on every unrelated state change will become noticeable. Move the two lookup objects outside the component and wrap `filteredLogs` in `useMemo`:
```js
// outside the component
const LOG_LEVEL_RANK = { debug: 0, info: 1, warn: 2, warning: 2, error: 3 };
const LOG_LEVEL_COLOR = { debug: "#8c8c8c", info: undefined, warn: "#d48806", warning: "#d48806", error: "#cf1322" };
// inside the component
const filteredLogs = useMemo(
() => logsInfo.filter(
(entry) => (LOG_LEVEL_RANK[entry.level] ?? 1) >= (LOG_LEVEL_RANK[logsLevel] ?? 1)
),
[logsInfo, logsLevel]
);
```
How can I resolve this? If you propose a fix, please make it concise.
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.
What
Why
Bottom logs panel was dominated by dev-internal debug lines and leaky enum reprs:
```
06:16:09 [info] [ThreadPool]: Processing Model : "".
06:16:09 [info] [ThreadPool]: Executing Model Node: ... Materialization: Materialization.EPHEMERAL ...
06:16:09 [info] [ThreadPool]: 1 of 2 ExecStatus.Success .................................... [ ExecStatus.OK ]
06:16:09 [info] [ThreadPool]: DONE PASS=0 WARN=0 ERROR=0 SKIP=0 TOTAL=0
```
Users couldn't tell what actually happened after an action. After this change, the same run reads:
```
06:16:09 [info] [ThreadPool]: Processing Model : "MModelA".
06:16:09 [info] [ThreadPool]: Executing Model Node: ... Materialization: TABLE ...
06:16:09 [info] [ThreadPool]: 1 of 2 SUCCESS MModelA .......... [ OK ]
06:16:09 [info] [ThreadPool]: DONE PASS=2 WARN=0 ERROR=0 SKIP=0 TOTAL=2
```
How
Backend (`backend/visitran/`)
Frontend (`frontend/src/ide/editor/no-code-model/no-code-model.jsx`)
Can this PR break any existing features?
Low risk:
Database Migrations
None.
Env Config
None.
Relevant Docs
None.
Related Issues or PRs
Dependencies Versions
No changes.
Notes on Testing
Screenshots
TBD
Checklist
I have read and understood the Contribution Guidelines.