Skip to content

UI improvements, bug fix -- seed run on duckdb#36

Merged
wicky-zipstack merged 10 commits intomainfrom
hover-text-copy
Apr 7, 2026
Merged

UI improvements, bug fix -- seed run on duckdb#36
wicky-zipstack merged 10 commits intomainfrom
hover-text-copy

Conversation

@Deepak-Gnanasekar
Copy link
Copy Markdown
Contributor

What

  • Added hover-to-copy functionality on no-code model table cells via new CopyableCell widget
  • Fixed DuckDB seed execution to surface clear error messages instead of cryptic NoneType AttributeError
  • Removed unused Custom Data section from Environment page

Why

  • Table cell data in the no-code model preview was not easily copyable — users had to manually select and copy text
  • DuckDB seed failures (e.g., reserved keyword table names like pivot.csv) raised 'NoneType' object has no attribute 'insert_csv_records' with no meaningful
    feedback to the user
  • Custom Data in Environment was stored in DB but never consumed during execution — dead UI that could confuse users

How

  • Created frontend/src/widgets/copyable-cell/ — a reusable component that shows a copy icon on hover and copies cell content to clipboard. Integrated into the
    no-code model table column renderer replacing raw <span> elements
  • Wrapped DuckDB DuckDbSeed.execute() with a null guard on db_connection and a try/except that raises SeedFailureException with the table name in the error
    message
  • Removed EnvCustomDataSection rendering, all customData state/handlers, fetchProjectByConnection, and related imports from NewEnv.jsx

Can this PR break any existing features. If yes, please list possible items. If no, please explain why.

  • CopyableCell: Additive change — wraps existing table cell content with hover-copy behavior, no change to data rendering logic
  • DuckDB seed fix: Only changes exception handling within the existing catch flow — errors now surface as SeedFailureException instead of raw AttributeError
  • Custom Data removal: Frontend-only change. Backend still accepts custom_data in API payloads and stores it (no migration needed), so no backward
    compatibility issue

Database Migrations

  • None

Env Config

  • None

Relevant Docs

  • N/A

Related Issues or PRs

  • N/A

Dependencies Versions

  • None

Notes on Testing

  • Hover over table cells in no-code model preview — copy icon should appear, clicking copies the value
  • Create a DuckDB project, upload a CSV with a reserved keyword name (e.g., pivot.csv), run seed — should show a descriptive error notification
  • Open Environment create/edit modal — confirm Custom Data section is no longer visible
  • Verify existing environment create/update flows still work without issues

Screenshots

Screenshot from 2026-04-02 16-58-06 Screenshot from 2026-04-02 16-57-19

Checklist

I have read and understood the Contribution Guidelines.

@Deepak-Gnanasekar Deepak-Gnanasekar self-assigned this Apr 2, 2026
@Deepak-Gnanasekar Deepak-Gnanasekar added bug Something isn't working enhancement New feature or request labels Apr 2, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR bundles three independent improvements: a new CopyableCell widget adds hover-to-copy behaviour on no-code model table cells; DuckDbSeed.execute() now guards against a null db_connection and wraps all downstream exceptions in a descriptive SeedFailureException; and NewEnv.jsx removes the dead Custom Data UI section and all its supporting state/handlers.

  • CopyableCell (frontend/src/widgets/copyable-cell/): new memoized component using Ant Design Tooltip, clipboard write, and a 2-second checkmark feedback state. Integrated into the no-code model column renderer with value={formattedValue} as both the value prop and children, ensuring the displayed text and clipboard payload are identical.
  • DuckDB seed fix (backend/visitran/adapters/duckdb/seed.py): null guard before calling insert_csv_records plus a try/except that re-raises any existing SeedFailureException and wraps everything else with the table name in the message — resolves the cryptic AttributeError for reserved-keyword CSV names like pivot.csv.
  • Notification fix (frontend/src/service/notification-service.js): markdown-typed error notifications previously had an empty-string title; now they surface as "Failed".
  • Custom Data cleanup (frontend/src/base/components/environment/NewEnv.jsx): removes customData state, projListDep, fetchProjectByConnection, EnvCustomDataSection, and the now-unused useMemo/generateKey imports — no backend changes required since the field is still accepted by the API.

Confidence Score: 5/5

PR is safe to merge — all changes are additive or clean removals of confirmed dead code with no backward-compatibility concerns

No P0 or P1 issues found. Both previous thread concerns (nested tooltip UX, value mismatch for Number cells) are resolved in this commit. The DuckDB null guard and exception wrapping are correct and follow the existing SeedFailureException pattern. The CopyableCell widget is well-structured with memo, proper timeout cleanup, and consistent value/children props. The NewEnv cleanup removes only confirmed dead UI. Confidence is 5/5.

No files require special attention

Important Files Changed

Filename Overview
backend/visitran/adapters/duckdb/seed.py Adds null guard on db_connection and try/except around insert_csv_records to surface a descriptive SeedFailureException instead of a raw AttributeError
frontend/src/base/components/environment/NewEnv.jsx Removes dead custom-data state, fetchProjectByConnection call, projListDep, and EnvCustomDataSection render — clean-up with no functional regressions
frontend/src/ide/editor/no-code-model/no-code-model.jsx Replaces raw span elements in column renderer with CopyableCell, passing formattedValue as both value prop and children so displayed and copied text are consistent
frontend/src/service/notification-service.js Fixes empty notification title for markdown-typed errors by setting finalMessage to 'Failed' instead of an empty string
frontend/src/widgets/copyable-cell/copyable-cell.css New CSS for CopyableCell widget: text-overflow ellipsis for cell text, flex layout for tooltip content, and word-break/pre-wrap for long values
frontend/src/widgets/copyable-cell/index.js New memoized CopyableCell widget with Ant Design Tooltip, clipboard copy on icon click or cell double-click, 2-second checkmark state, and proper timeout cleanup on unmount

Sequence Diagram

sequenceDiagram
    participant U as User
    participant T as CopyableCell
    participant C as Clipboard API

    U->>T: Hover over table cell
    T-->>U: Show Tooltip with value + copy icon
    U->>T: Click copy icon (or double-click cell text)
    T->>C: navigator.clipboard.writeText(textValue)
    C-->>T: Resolved / Rejected (silently caught)
    T-->>U: Show CheckOutlined for 2 s, then reset

    note over U,C: DuckDB Seed Error Flow
    participant B as DuckDbSeed.execute()
    participant D as insert_csv_records()
    participant E as SeedFailureException

    U->>B: Trigger seed run
    B->>B: null guard — db_connection is None?
    B->>D: insert_csv_records(abs_path, table_name)
    D-->>B: Exception (e.g. reserved keyword table name)
    B->>E: raise SeedFailureException(table_name + err)
    E-->>U: Descriptive error notification displayed
Loading

Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.
Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

Reviews (3): Last reviewed commit: "lint fix" | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@wicky-zipstack wicky-zipstack left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@tahierhussain tahierhussain left a comment

Choose a reason for hiding this comment

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

LGTM

@wicky-zipstack wicky-zipstack merged commit dc4c347 into main Apr 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants