Commit Graph
5 Commits
Author SHA1 Message Date
Charis 628473b3eb refactor(studio): extract notebook query-cell logic and give log cells display settings (#49075)
Final PR of the stack. #49069, #49070, #49072 and #49074 have merged, so
this now targets `master` directly.

**Rebased onto latest `master`**, which includes the centralized
result-rendering work (#49096). See "Conflict resolution" below.

## What's left after master's own fixes

`QueryCell` was written for database cells and adapted to log cells
afterwards. Master has since fixed most of it directly:
`handleUpdateCell` no longer bails on a non-database cell, the cell's
own binding is read via `getQuerySourceBinding`, and
`database_identifier` / `time_range` propagate across a source change.

What remains:

- **`display` was only passed for database cells**, so the `view` field
on `log_cell` stayed unreachable and a logs query could never be
charted. That is the one behavioral fix left in this PR.
- The per-backend branching is inline and untested.

## What changed

Per-backend logic moves into `QueryCell.utils.ts`, where it is
unit-tested: `changeCellSource`, `setCellSql`, `cloneQueryCell`,
`getCellDisplay`, `toQueryModel`. Each narrows on the cell tag exactly
once, so the SQL brand and the backend's parameters stay correlated
rather than being re-derived at each call site. `cloneQueryCell` also
rebuilds the chart's series array, which valtio hands over as `readonly
string[]`.

`NotebookEditor` renders through `isQueryCell` (#49069) rather than a
tag switch, so a new backend gets picked up by classifying it in
`CELL_KINDS` instead of by remembering to add a `case`.

## Conflict resolution

Two rounds of master's work landed in this file set.

**`QueryCell/index.tsx` (master's own rework).** `changeCellSource`
**subsumes the four source-change branches** master had inline, each
covered by a test:

| Master's branch | Test |
|---|---|
| database → database (replica change) | `keeps the query when only the
database changes` |
| logs → logs (time-range change) | `keeps the query when only the log
time range changes` |
| database → logs | `carries the query text over when moving from the
database to logs` |
| logs → database | `carries the query text over and restores a default
row limit …` |

Two improvements fall out of consolidating them:

- A **logs → database** move now keeps the selected replica; pinned by
`applies the selected database when moving from logs to the database`.
- The row-limit default is **named** rather than a hard-coded `100`.
`Explorer/utils.ts` now shares `DEFAULT_CELL_ROW_LIMIT` with
`createQueryCellSkeleton`, so cell creation and backend conversion can't
drift.

Untouched from master: `snap.updateCell`, `AddCellDropdown`,
`MoveCellDropdownContent`, the `SortableSection` grip props, and
`NotebookEditor`'s add-cell buttons, skeletons, `reorderCells` and
`insertCellAfter`.

**Centralized result rendering (#49096).** That PR moved
`QueryCell/QueryResultChart.tsx` up to `Explorer/`, split
`QueryResultTable` into `QueryResultError`, and added
`QueryResultRenderer`. Since this PR removes `QueryChartConfig`, the
type swap had to follow the move and also reach `QueryResultRenderer`,
which is new and referenced the removed type. `QueryResultRenderer`,
`QueryResultError` and `DataGridResults` are otherwise untouched — the
empty/error-state centralization is fully preserved, and `QueryEditor`
still renders through it.

## Behavior worth a second opinion

`changeCellSource` **carries the query text across a backend change**
and rebrands it. This is probably not what a user wants — Postgres SQL
and logs SQL are separate dialects over separate schemas, so a
carried-over query will usually fail to run, and the rebrand asserts a
dialect the text was never written in.

Keeping it for now because it destroys nothing and needs no confirmation
prompt. The tradeoff is written up at the function. Worth revisiting
once we know whether people switch source to port an existing query or
to start a fresh one — if it's the latter, clearing the body behind a
confirmation is the better answer.

Results *are* dropped on a backend change, since another engine returns
unrelated columns.

## Incidental

`Explorer/types.ts` drops `QueryChartConfig`, which duplicated the wire
schema's `ChartConfig` field for field. `chart` stays persisted
alongside `view`, so switching to the table and back returns the user's
chart settings rather than rebuilding them.

## Verification

Typecheck, Prettier, and the lint ratchet clean. 1013 tests pass across
`state/`, the Explorer surfaces, notebooks, query sources, `data/sql`,
the SQL editor, and `components/ui`; 13 of them are new coverage for the
extracted helpers.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Improved notebook cell rendering with more consistent handling of
query and markdown cells.
- Query cells now preserve SQL, source settings, display preferences,
chart configuration, and query results when edited or switched between
sources.
  - Added a default limit of 100 rows for applicable database queries.

- **Bug Fixes**
- Prevented stale query results from carrying over when changing query
sources.
- Improved chart configuration consistency across query results and
display settings.

- **Tests**
- Added comprehensive coverage for query-cell updates, source
transitions, SQL changes, display state, and chart data.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-14 09:42:18 -04:00
Joshen Lim 93d9d80535 joshen/fe 4149 explorer support adding removing cells in notebook (#49037)
## Context

Related to Explorer/Notebooks - this PR adds the functionality to add /
remove cells

Separately - am thinking we can shift a lot of the "cell update" logic
into notebook-state, mainly so that each UI component doesn't need to be
aware of the notebook's `cells` but just its own cell.

I'll do it separately though to prevent bloating this PR, already left
comments where i think can be refactored

<img width="250" alt="image"
src="https://github.com/user-attachments/assets/91aae5f9-07b9-4327-8e21-8e9d71098aa6"
/>
<img width="250" alt="image"
src="https://github.com/user-attachments/assets/677de208-5c52-4155-90c6-bc0879070693"
/>
<img width="250" alt="image"
src="https://github.com/user-attachments/assets/17c3dae7-87bc-40bb-bfa0-15df101006ce"
/>
<img width="1091" height="262" alt="image"
src="https://github.com/user-attachments/assets/a09c3d23-696a-4b55-a1f8-4602e56a60dc"
/>



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## New Features
- Add query and Markdown cells directly within notebooks.
- Move, remove, and reorder cells using drag-and-drop or cell controls.
- Add cells from empty states and notebook toolbar actions.
- Edit Markdown cells, mark edits as complete, and see placeholders for
empty content.

## Improvements
- Cell controls and hover interactions are more consistent and
responsive.
- Moving cells is disabled at the top or bottom of a notebook.
- Sample cells now use standardized content and formatting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-14 00:17:29 +07:00
Saxon Fletcher ace9422bfd refactor(studio): share Explorer query editor (#49041)
## Stack

Depends on #49027. Followed by #49038.

## Summary

- extract a controlled `QueryEditor` from the existing notebook query
cell
- reuse it from `QueryCell`, leaving notebook persistence and
sortable-block behavior in the adapter
- make table/chart result settings controlled so other query surfaces
can share them
- persist notebook SQL on editor blur and query execution

## To test

1. Open a notebook query cell, edit and rename it, then run the query
and confirm results appear.
2. Switch between table and chart results and confirm notebook
move/delete actions still work.

## Why

Notebooks, query tabs, and future chat tabs need consistent query
actions and result rendering without duplicating the notebook
implementation.

## Impact

This is primarily a refactor of the existing notebook query experience.
It introduces no new query-tab routes or source-selection behavior.

## Validation

- fresh non-incremental Studio TypeScript check
- focused NotebookEditor component tests

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added a shared query editor with SQL editing, execution, validation,
visibility controls, editable titles, row limits, and loading/error
states.
* Added table and chart result views, including customizable bar and
line charts.
* Added support for switching display modes and updating chart settings.

* **Improvements**
  * Improved query result handling and display-setting updates.
  * Repositioned the logarithmic-scale tooltip for better visibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Joshen Lim <joshenlimek@gmail.com>
2026-08-13 17:59:30 +07:00
Joshen Lim 09894096e2 Add log scale support to query result chart (#49049)
## Context

Related to Explorer/Notebooks - Adds support for log scale in the charts
<img width="980" height="477" alt="image"
src="https://github.com/user-attachments/assets/b3764023-bd9a-4edd-92d4-1b61df93b97e"
/>


Prevents setting to log scale if y axis has yet to be selected, or if
the data set contains non positive values
<img width="393" height="188" alt="image"
src="https://github.com/user-attachments/assets/57695d9e-9724-47ed-aa2a-c104c16837b2"
/>
<img width="408" height="159" alt="image"
src="https://github.com/user-attachments/assets/2290bb69-0ddb-4449-afad-aabae6ace669"
/>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
  - Added logarithmic Y-axis scaling for line and bar charts.
  - Added logarithmic tick formatting and appropriate chart bounds.
  - Added contextual guidance when logarithmic scaling is unavailable.

- **Bug Fixes**
- Automatically switches charts back to linear scaling when data
contains values that cannot support logarithmic scaling.
  - Prevented unnecessary stacking for single-series area charts.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-13 16:42:28 +07:00
Joshen Lim d434c63bad joshen/fe 4150 explorer query cells result display settings (#49003)
## Context

Related to Explorer / Notebooks - this adds chart functionality for the
Query cells
<img width="250" alt="image"
src="https://github.com/user-attachments/assets/4ea37c14-87dc-4c43-ba7f-cb9436085c81"
/>

Query results can be rendered as either bar or line chart - using the
chart packages from `ui-patterns`
[NOTE]: For design team reviewers - am patching the chart packages to be
agnostic to the `timestamp` property within the provided data set. Would
love to use this component from a consistency POV instead of the old
`BarChart` component we have.

Have intentionally omitted log scale functionality from this PR - will
have that separately 🙏

<img width="999" height="483" alt="image"
src="https://github.com/user-attachments/assets/14356ee4-c658-4fd1-90e0-17c38dac4822"
/>
<img width="988" height="478" alt="image"
src="https://github.com/user-attachments/assets/cd0ca088-9a03-4aa3-9884-17bc36d3cabf"
/>



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Added chart views for notebook query results, including bar and line
charts.
- Added display settings for selecting X/Y columns, chart type, scale,
cumulative values, and label visibility.
  - Added configurable X-axis support for charts.
  - Display preferences are saved with each notebook cell.

- **Improvements**
  - New database cells default to table view.
  - Chart results better handle varied data types.
- Empty results and incomplete chart settings now display clear
placeholders.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-13 10:09:54 +07:00