FIX: Reduce logs panel noise and make execution messages readable (OR-1457)#59
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 | Fixed counter comparisons to use .value and elif chains; logic is correct, though no code path in visitran.py currently produces a "WARN" or "SKIPPED" end_status so those counters remain 0. |
| backend/visitran/events/types.py | Single-line demotion of UsingCachedObject from InfoLevel to DebugLevel to suppress per-query cache spam from the UI. Change is correct and intentional. |
| backend/visitran/visitran.py | Replaced all str(ExecStatus.X) usages with ExecStatus.X.value and fixed ProcessingModel class name lookup; all six call sites updated consistently. |
| frontend/src/ide/editor/no-code-model/no-code-model.jsx | Added log-level Select filter with localStorage persistence, per-row severity tinting, useMemo for filteredLogs, and improved socket cleanup with explicit disconnect. Changes are well-structured. |
Sequence Diagram
sequenceDiagram
participant BE as Backend (visitran.py)
participant EV as events/printer.py
participant SK as WebSocket
participant FE as NoCodeModel (JSX)
BE->>BE: run node (success/fail)
BE->>EV: BaseResult(end_status=ExecStatus.OK.value / Fail.value)
BE->>EV: parse_and_fire_reports()
EV->>EV: compare end_status against OK/Fail/Warn/Skipped .value
EV->>SK: fire_event(SummaryReport, EndSummaryReportCounts)
SK-->>FE: logs:{socketSessionId} { level, message }
FE->>FE: setLogsInfo([...old, { level, message }])
FE->>FE: filteredLogs = useMemo(filter by LOG_LEVEL_RANK[logsLevel])
FE->>FE: render rows tinted by LOG_LEVEL_COLOR[el.level]
note over FE: logsLevel persisted in localStorage (default: info)
Prompt To Fix All With AI
This is a comment left during a code review.
Path: backend/visitran/events/printer.py
Line: 91-96
Comment:
**`warn_count` and `skip_count` will still always be zero**
The comparisons against `ExecStatus.Warn.value` ("WARN") and `ExecStatus.Skipped.value` ("SKIPPED") are now logically correct, but a grep of `visitran.py` shows that no code path ever sets `end_status` to either of those values — only `ExecStatus.OK.value` and `ExecStatus.Fail.value` are assigned. As a result, the DONE line will still always show `WARN=0 SKIP=0` regardless of what actually happened. This is a pre-existing gap that the counter fix doesn't fully close; if warn/skip paths are planned, the corresponding `end_status` assignments in `visitran.py` will need to be added.
How can I resolve this? If you propose a fix, please make it concise.Reviews (3): Last reviewed commit: "fix: use antd theme token for log-panel ..." | Re-trigger Greptile
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Socket cleanup fix (d281ec8): The socket |
|
LGTM overall |
Deepak-Gnanasekar
left a comment
There was a problem hiding this comment.
Review — APPROVED
Reviewed all 4 changed files (+94 / -43). Clean, targeted fixes with no new logic issues introduced.
Backend
printer.py — The root cause of PASS=0 WARN=0 ERROR=0 SKIP=0 TOTAL=0 was comparing end_status against ExecStatus.Success/ExecStatus.Error when it's actually set to OK/Fail. Fix is correct. The if → elif tightening is also appropriate since end_status values are mutually exclusive.
types.py — UsingCachedObject demotion from InfoLevel to DebugLevel is a clean one-liner. Users who need it can switch to "All logs" via the new dropdown.
visitran.py — All str(ExecStatus.X) → ExecStatus.X.value replacements are consistent across all 5 call sites (models, tests, seeds, snapshots). The hasattr guard on node.materialization.value is a good defensive choice. getattr(cls, "__name__", "") or file_name correctly handles dynamic no-code model classes that lacked a meaningful __str__.
Frontend (no-code-model.jsx)
LOG_LEVEL_RANK/LOG_LEVEL_COLORare correctly defined outside the component (no re-creation on render).filteredLogswrapped inuseMemowith correct[logsInfo, logsLevel]deps.localStoragepersistence for log level is straightforward.- Socket cleanup fix is a genuine improvement: capturing
socketSessionIdvia closure ensures cleanup targets the right listener, andnewSocket.disconnect()properly tears down the connection on unmount. The old code referenced the outer-scopesessionId(cookie value) which could diverge from the server-assigned session. dangerouslySetInnerHTMLusage is pre-existing andparseLogsanitizes viaDOMPurify— no new XSS surface.
Minor note (non-blocking)
- If a log message arrives before the
session_idevent (unlikely but theoretically possible with race conditions), it would be silently dropped. Current behavior is fine since the socket listener is only registered aftersession_idfires anyway.
LGTM — safe to merge.
tahierhussain
left a comment
There was a problem hiding this comment.
@abhizipstack Can you please add screenshots for the UI changes addressed in this PR?
Replaced hardcoded #f0f0f0 border with token.colorBorderSecondary so the log-level dropdown separator renders correctly in both light and dark themes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
No screenshots available — the log-level filter and severity-tinted rows require a WebSocket connection for live log streaming, which is not available in the React dev server proxy setup. The feature works in production where nginx handles WebSocket forwarding.
UI changes:
Checklist
TBD
Checklist
I have read and understood the Contribution Guidelines.