Skip to content

feat: track model execution status with failure icon in explorer#54

Open
abhizipstack wants to merge 3 commits intomainfrom
feat/model-execution-status-tracking
Open

feat: track model execution status with failure icon in explorer#54
abhizipstack wants to merge 3 commits intomainfrom
feat/model-execution-status-tracking

Conversation

@abhizipstack
Copy link
Copy Markdown
Contributor

What

  • Add RunStatus enum and 4 new fields to ConfigModels: run_status, failure_reason, last_run_at, run_duration
  • New migration 0003_add_model_run_status.py
  • Update file_explorer.load_models() to return status fields for each model
  • Add _update_model_status() to DAG executor in visitran.py — called before execution (RUNNING), after success (SUCCESS), and on exception (FAILED)
  • Update execute/views.py to return 400 on DAG execution failures
  • Fix clear_cache decorator to re-raise exceptions instead of silently swallowing them
  • Frontend: add colored status dot badges next to model names in the explorer tree (blue=running, green=success, red=failed)
  • Frontend: Popover on hover over failed status shows full error message (scrollable) and last run timestamp
  • Frontend: trigger explorer refresh via setRefreshModels after runTransformation succeeds or fails

Why

After a model build/run, users had no visual indication of which models succeeded or failed. They had to manually open each model to check. This change shows failure icons directly in the explorer so users can quickly identify broken models and fix them via prompts or manual edits.

How

Status is tracked at the DAG node level (not API level). When the DAG executor processes each node, it updates the corresponding ConfigModels row in the database with the current status. This ensures accurate per-model status even when Model A succeeds but downstream Model B fails — Model A is marked SUCCESS while Model B is marked FAILED.

The frontend file_explorer API already returns the full model tree — we just add the new status fields to each model entry. The explorer renders a small colored dot inline before the model name based on run_status. Failed models show a Popover on hover with the full failure_reason and last_run_at.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

Low risk:

  • The 4 new DB fields are nullable with sensible defaults (NOT_STARTED), so existing models continue to work
  • Status tracking wraps the existing execution logic with try/except — if status update fails, the error is logged but does not interrupt execution
  • The clear_cache decorator fix actually addresses a pre-existing silent bug where execution errors were being swallowed and returning NoneType to DRF (causing 500 errors instead of proper error responses)
  • CLI mode (no Django) is handled — ConfigModels import is wrapped in try/except so visitran CLI still works without Django
  • Frontend badge rendering is additive — models without status fields render unchanged

Database Migrations

  • backend/backend/core/migrations/0003_add_model_run_status.py — adds 4 nullable fields to configmodels table (run_status, failure_reason, last_run_at, run_duration)

Env Config

None

Relevant Docs

None

Related Issues or PRs

None

Dependencies Versions

No changes

Notes on Testing

Tested locally with Python 3.11:

  • Migration applied cleanly
  • Model RunStatus choices verified via Django shell
  • DAG execution correctly updates status per-node (RUNNING → SUCCESS/FAILED)
  • Explorer renders status dots correctly with inline vertical-align
  • Popover on failed models shows full error message with scroll
  • clear_cache fix verified — MultipleColumnDependency exception now returns proper 400 error response instead of 500 NoneType

Screenshots

TBD — will add after testing locally

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

Implement model-level execution status tracking (NOT_STARTED, RUNNING,
SUCCESS, FAILED) at DAG node level with visual indicators in the file
explorer. Status is tracked per individual DAG node, ensuring accurate
status for each model even when downstream dependencies fail.

Backend changes:
- Add RunStatus enum and 4 new fields to ConfigModels (run_status,
  failure_reason, last_run_at, run_duration)
- Migration 0003_add_model_run_status
- Update file_explorer.load_models to return status fields
- Add _update_model_status to DAG executor — called before execution
  (RUNNING), after success (SUCCESS), and on exception (FAILED with
  reason)
- Update execute/views.py to return 400 on DAG execution failures
- Fix clear_cache decorator to re-raise exceptions instead of
  swallowing them silently

Frontend changes:
- Add getModelRunStatus helper to render colored dot badges next to
  model names in the explorer tree
- Running: blue, Success: green, Failed: red
- Show Popover on hover over failed status with full error message
  and last run timestamp
- Trigger explorer refresh via setRefreshModels after runTransformation
  succeeds or fails

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@abhizipstack abhizipstack requested review from a team as code owners April 10, 2026 09:29
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR adds per-model execution status tracking — a RunStatus enum, four new DB fields, a migration, DAG-level status updates in the executor, and frontend status dot badges with hover popovers in the explorer tree.

  • P1 — connection leak in failure path: close_db_connection() is only called in the success branch of execute_run_command; every failed DAG run leaves the connection open. Should move to a finally block.

Confidence Score: 4/5

Safe to merge after fixing the DB connection leak in the execute_run_command failure path.

One P1 defect: close_db_connection() is never called when execute_visitran_run_command raises, leaking a connection on every run failure. All other changes (migration, model, status tracking, frontend badges, cache decorator fix) are well-structured and low risk.

backend/backend/core/routers/execute/views.py — missing close_db_connection() in the exception path.

Important Files Changed

Filename Overview
backend/backend/core/routers/execute/views.py Adds try/except around DAG execution to return 400 on failure, but omits close_db_connection() in the failure path — leaks the DB connection on every run error.
backend/visitran/visitran.py Adds _update_model_status() with RUNNING/SUCCESS/FAILED transitions and run_duration tracking; logic is sound.
backend/backend/utils/cache_service/decorators/cache_decorator.py Adds raise to re-propagate exceptions from the view function instead of silently swallowing them and returning None — correct fix.
backend/backend/application/file_explorer/file_explorer.py Extends the model tree payload with run_status, failure_reason, last_run_at, and run_duration; handles missing model lookup gracefully with getattr defaults.
backend/backend/core/migrations/0003_add_model_run_status.py New migration adds 4 nullable fields (run_status with default, failure_reason, last_run_at, run_duration) — non-breaking, backward-compatible.
backend/backend/core/models/config_models.py Adds RunStatus TextChoices enum and 4 new nullable DB fields consistent with the migration.
frontend/src/ide/explorer/explorer-component.jsx Adds getModelRunStatus helper rendering colored dot badges (RUNNING/SUCCESS/FAILED) with Tooltip/Popover; triggers explorer refresh on both success and error paths.
frontend/src/ide/editor/no-code-model/no-code-model.jsx Calls setRefreshModels(true) in both then/catch of runTransformation so the explorer badge updates after each run attempt.

Sequence Diagram

sequenceDiagram
    participant FE as Frontend
    participant View as execute_run_command
    participant App as ApplicationContext
    participant DAG as Visitran.execute_graph
    participant DB as ConfigModels (DB)
    participant Exp as file_explorer API

    FE->>View: POST /execute/run
    View->>App: execute_visitran_run_command()
    loop Each DAG node
        App->>DAG: execute_graph()
        DAG->>DB: _update_model_status(RUNNING)
        DAG->>DAG: run_model()
        alt Success
            DAG->>DB: _update_model_status(SUCCESS, run_duration)
        else Failure
            DAG->>DB: _update_model_status(FAILED, failure_reason, run_duration)
            DAG-->>View: raise exception
        end
    end
    View->>App: close_db_connection()
    View-->>FE: 200 success / 400 failed
    FE->>FE: setRefreshModels(true)
    FE->>Exp: GET /file_explorer
    Exp->>DB: fetch models with run_status fields
    Exp-->>FE: model tree + status fields
    FE->>FE: render colored dot badges
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: backend/backend/core/routers/execute/views.py
Line: 60-70

Comment:
**DB connection leaked on execution failure**

`close_db_connection()` is only called in the happy path. When `execute_visitran_run_command` raises, the `except` branch returns immediately without ever closing the connection, leaking it every time a DAG run fails. This should use a `finally` block:

```suggestion
    try:
        app.execute_visitran_run_command(current_model=file_name, environment_id=environment_id)
        app.backup_current_no_code_model()
        logger.info(f"[execute_run_command] Completed successfully for file_name={file_name}")
        _data = {"status": "success"}
        return Response(data=_data)
    except Exception:
        logger.exception(f"[execute_run_command] DAG execution failed for file_name={file_name}")
        _data = {"status": "failed", "error_message": "Model execution failed. Check server logs for details."}
        return Response(data=_data, status=status.HTTP_400_BAD_REQUEST)
    finally:
        app.visitran_context.close_db_connection()
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (3): Last reviewed commit: "fix: satisfy prettier multiline formatti..." | Re-trigger Greptile

wicky-zipstack added a commit that referenced this pull request Apr 13, 2026
- Remove unused .project-list-card-clickable-content CSS class
- Reduce !important usage in schema select styles, increase specificity instead
- Replace duplicate API call with explorerService.setProjectSchema
- Replace message.success/error with useNotificationService for consistency
- Show error InfoChip when schema list is empty instead of hiding dropdown
- Revert project_schema max_length change (will be added to PR #54 with migration)
- Drop optional ConfigModels import; it's always present in the OSS build
- Track run_duration via time.monotonic() and persist on SUCCESS/FAILED
- Skip RUNNING update for non-executable parent nodes so they don't get
  stuck with a permanent blue badge in selective execution
- Raise explicit errors when session/project_id missing instead of silent no-op
- Stop returning raw exception strings from execute_run_command (CodeQL)
- Refresh explorer tree on context-menu run failure so FAILED badge appears
- Move getModelRunStatus to module scope and use antd theme tokens instead
  of hardcoded hex colors

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@abhizipstack
Copy link
Copy Markdown
Contributor Author

Addressed all review feedback in 55332f7:

Greptile P1s

  • Non-executable parent nodes stuck in RUNNING → wrapped the RUNNING status update with if is_executable so parents imported only for DAG resolution no longer get a permanent blue badge.
  • Context-menu run failures not refreshing the explorer → handleModelRun's .catch() now calls getExplorer(projectId) and setRefreshModels(true), so the red FAILED badge appears immediately.

Greptile P2

  • run_duration now populated via time.monotonic() around each node; persisted on SUCCESS and both FAILED branches.

@tahierhussain

  • Dropped the conditional ConfigModels import and the if ConfigModels else "FAILED" fallbacks.
  • Missing session/project_id now raise ValueError (still caught + logged via logging.exception).
  • getModelRunStatus hoisted to module scope; uses antd theme tokens (colorInfo / colorError / colorSuccess / colorTextSecondary) instead of hardcoded hex.

CodeQL

  • Response no longer includes the raw exception string; full traceback is logged server-side and a generic error message is returned to the client.


import ibis
import networkx as nx
from django.utils import timezone
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unconditional Django imports break CLI mode

from django.utils import timezone (and from backend.core.models.config_models import ConfigModels at line 76) are top-level, unconditional imports. Before this PR, visitran.py had no Django imports. Any caller that uses visitran in CLI mode without Django configured will now get a ModuleNotFoundError or django.core.exceptions.ImproperlyConfigured at module load time — before any try/except can help.

The PR description explicitly states "CLI mode (no Django) is handled — ConfigModels import is wrapped in try/except", but neither import has a guard in the code. The try/except inside _update_model_status only catches runtime failures, not import-time failures.

To protect CLI mode as stated, wrap both imports:

try:
    from django.utils import timezone
    from backend.core.models.config_models import ConfigModels
    _DJANGO_AVAILABLE = True
except ImportError:
    _DJANGO_AVAILABLE = False
    timezone = None
    ConfigModels = None

Then gate _update_model_status on _DJANGO_AVAILABLE.

Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/visitran/visitran.py
Line: 19

Comment:
**Unconditional Django imports break CLI mode**

`from django.utils import timezone` (and `from backend.core.models.config_models import ConfigModels` at line 76) are top-level, unconditional imports. Before this PR, `visitran.py` had no Django imports. Any caller that uses visitran in CLI mode without Django configured will now get a `ModuleNotFoundError` or `django.core.exceptions.ImproperlyConfigured` at module load time — before any try/except can help.

The PR description explicitly states *"CLI mode (no Django) is handled — `ConfigModels` import is wrapped in try/except"*, but neither import has a guard in the code. The try/except inside `_update_model_status` only catches runtime failures, not import-time failures.

To protect CLI mode as stated, wrap both imports:

```python
try:
    from django.utils import timezone
    from backend.core.models.config_models import ConfigModels
    _DJANGO_AVAILABLE = True
except ImportError:
    _DJANGO_AVAILABLE = False
    timezone = None
    ConfigModels = None
```

Then gate `_update_model_status` on `_DJANGO_AVAILABLE`.

How can I resolve this? If you propose a fix, please make it concise.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +60 to +70
try:
app.execute_visitran_run_command(current_model=file_name, environment_id=environment_id)
app.visitran_context.close_db_connection()
app.backup_current_no_code_model()
logger.info(f"[execute_run_command] Completed successfully for file_name={file_name}")
_data = {"status": "success"}
return Response(data=_data)
except Exception:
logger.exception(f"[execute_run_command] DAG execution failed for file_name={file_name}")
_data = {"status": "failed", "error_message": "Model execution failed. Check server logs for details."}
return Response(data=_data, status=status.HTTP_400_BAD_REQUEST)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 DB connection leaked on execution failure

close_db_connection() is only called in the happy path. When execute_visitran_run_command raises, the except branch returns immediately without ever closing the connection, leaking it every time a DAG run fails. This should use a finally block:

Suggested change
try:
app.execute_visitran_run_command(current_model=file_name, environment_id=environment_id)
app.visitran_context.close_db_connection()
app.backup_current_no_code_model()
logger.info(f"[execute_run_command] Completed successfully for file_name={file_name}")
_data = {"status": "success"}
return Response(data=_data)
except Exception:
logger.exception(f"[execute_run_command] DAG execution failed for file_name={file_name}")
_data = {"status": "failed", "error_message": "Model execution failed. Check server logs for details."}
return Response(data=_data, status=status.HTTP_400_BAD_REQUEST)
try:
app.execute_visitran_run_command(current_model=file_name, environment_id=environment_id)
app.backup_current_no_code_model()
logger.info(f"[execute_run_command] Completed successfully for file_name={file_name}")
_data = {"status": "success"}
return Response(data=_data)
except Exception:
logger.exception(f"[execute_run_command] DAG execution failed for file_name={file_name}")
_data = {"status": "failed", "error_message": "Model execution failed. Check server logs for details."}
return Response(data=_data, status=status.HTTP_400_BAD_REQUEST)
finally:
app.visitran_context.close_db_connection()
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/backend/core/routers/execute/views.py
Line: 60-70

Comment:
**DB connection leaked on execution failure**

`close_db_connection()` is only called in the happy path. When `execute_visitran_run_command` raises, the `except` branch returns immediately without ever closing the connection, leaking it every time a DAG run fails. This should use a `finally` block:

```suggestion
    try:
        app.execute_visitran_run_command(current_model=file_name, environment_id=environment_id)
        app.backup_current_no_code_model()
        logger.info(f"[execute_run_command] Completed successfully for file_name={file_name}")
        _data = {"status": "success"}
        return Response(data=_data)
    except Exception:
        logger.exception(f"[execute_run_command] DAG execution failed for file_name={file_name}")
        _data = {"status": "failed", "error_message": "Model execution failed. Check server logs for details."}
        return Response(data=_data, status=status.HTTP_400_BAD_REQUEST)
    finally:
        app.visitran_context.close_db_connection()
```

How can I resolve this? If you propose a fix, please make it concise.

@@ -0,0 +1,53 @@
from django.db import migrations, models
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.

@abhizipstack Can you please address this comment in this PR?

Context: We need to make the required update in the project_details model and add the corresponding migration file changes as well.

#55 (comment)

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.

3 participants