Skip to content

feat: show rule-based trends in ResultGrid - #613

Merged
emrberk merged 25 commits into
mainfrom
feat/grid-highlight-rules
Sep 30, 2026
Merged

emrberk merged 25 commits into
mainfrom
feat/grid-highlight-rules

Conversation

@emrberk

@emrberk emrberk commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Highlight rules for notebook result grids

Result grids in notebook cells can now color cells and rows by rule, so a notebook can serve as a live dashboard: watchlists that flash on price moves, threshold breaches that stay colored, heatmap columns, and event tapes that flash new rows.

What you can do

  • Open "Highlight rules" from a grid cell's menu, or from the gear in the maximized header. Rules are saved per cell and apply to every result grid of that cell by column name. They survive SQL edits, export, and import.
  • Compare with the previous refresh. Pick the identity columns that name the same row across refreshes, for example symbol and side. Then use > previous, < previous, changed, or changed by at least an absolute or percent amount. Matching cells flash, and > previous / < previous show a ▲ or ▼ glyph. Timestamps compare as instants.
  • Flash new rows. A "new row" rule paints any row whose identity was not in the previous result.
  • Compare with a fixed value. >, ≥, <, ≤, =, between, is null, contains, and matches a regular expression. Text comparisons ignore case. A rule can target one column or all numeric columns.
  • Band values into steps. Each step means "from this value up". A value takes the highest step it reaches; a base color covers everything below the first step.
  • Gradient. A between rule with the gradient fill shades every cell of the column from one color at the low bound to another at the high bound, mixed in between and clamped beyond the ends. Leave a bound empty for auto: it follows the column's current minimum or maximum on every refresh, so a heatmap column keeps its full spread as data moves.
  • Choose how a match shows. Flash fades before the next refresh lands; Permanent stays until the next result. A rule can color its own cell or the whole row.
  • Order matters. Rules evaluate top to bottom. The first matching rule colors a cell. A row rule listed first paints the whole row; a cell rule listed first keeps its cell. Rules can be reordered, disabled, and removed.
  • Colors come from ten theme-aware hues. Red and green are the loss and gain pair.

Custom auto-refresh intervals

Auto-refresh is no longer limited to five presets. A cell or a notebook can poll at any fixed interval from 50ms to 60m.

  • New presets: 250ms, 500ms, 1s, 5s, 10s, 30s, 1m, next to Auto and Off.
  • Custom interval: the interval menu ends with an input. Type a number with a unit (ms, s or m), for example 750ms or 15m, and press Enter. Spaces and case do not matter. A value outside 50ms–60m shows a hint and is not applied. A custom interval in use appears in the menu as a checked option.
  • Fixed cadence: a fixed interval now runs on its own loop. Each fetch starts one interval after the previous fetch started, so the cadence does not drift with query latency. Auto keeps the adaptive loop.

Behavior that follows the interval

Fast intervals need a different UI from a 30s dashboard, so several things now scale with the cell's effective interval (the cell override, or the notebook default):

  • Flash duration is 80% of the interval, capped at 1s. A changed rule at 250ms flashes for 200ms, so each tick reads on its own instead of merging into one long highlight.
  • Chart animation is off below 500ms. Line, area, step and scatter series never animate; bar, stacked bar, pie and candlestick animate only at 500ms and slower.
  • Refresh button: a refresh keeps the button busy (spinner, no clicks) for at least 1s. A cell polling at 1s or faster shows one continuous spinner instead of a flicker, and a manual refresh waits until the last one is a moment old.

Agent and MCP

The assistant and the MCP bridge can set or clear a cell's rules with set_cell_highlight_config, and apply_notebook_state carries highlight_config per cell. Notebook snapshots include the rules.

set_cell_autorefresh, set_notebook_autorefresh and the auto_refresh / auto_refresh_default fields of apply_notebook_state accept any interval matching ^[1-9][0-9]*(ms|s|m)$ within 50ms–60m, in place of the previous enum. Validation errors name the accepted form.

Also in this PR

  • Chart settings and highlight rules share one drawer shell. Drawers animate on close and keep their draft across maximize and restore. Focus returns to the opener after the slide-out ends.
  • Settings drawers cover the cell body, so the grid stays visible next to the panel.
  • Menu items no longer shrink when a menu is taller than the viewport; the menu scrolls instead.
  • Inputs inside menus (the column picker search, the custom interval) share one style.
  • The Metrics color palette moved to a shared component.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Web Console deploy preview

Preview Commit Logs
https://pr-613--web-console.netlify.app 012e081 build log

emrberk and others added 19 commits September 25, 2026 09:54
A row rule counts as a match for every cell of its row, so the first
matching rule in the list wins per cell. A row rule listed first paints
the whole row; a cell rule listed first keeps its cell and the row rule
fills the rest. Schema and prompt describe the same order.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The drawer shell keeps its nodes for a slide-out before unmounting, and
both drawers let the shell own visibility. A maximize or restore remounts
the cell, so the open state and the unsaved draft (rules, expanded rule,
chart config) live in a per-cell session store and come back in place
without replaying the entrance.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Row rule listed after a matching cell rule on the same column now paints the rest of the row
- Between bounds accept auto (null): the engine reads the column's current min/max on every result; new between rules default to auto
- Text equality ignores case like contains; previous-result rules compare timestamps as instants
- Wire default unit is percent like the drawer; wire rejects a flat gradient and a reversed range
- Trend store moves into NotebookProvider behind ResultTrendContext, so maximize/restore keeps the baseline
- Trend capture has one writer, run from the cells store before render; the grid only reads
- Field validation lives once in the engine and serves the drawer and the wire; loaded configs pass a deep structural guard
- ResultGridPanel event-bus effect gets deps; the highlight drawer writes its draft from setters; the tool reports its own AI status

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A `newRow` rule paints the whole row of a row whose identity key was not in
the previous result. It carries only a color and a display, needs no column,
and defaults to a flash. The drawer offers it as "new row" under the
previous-result conditions and hides the column and applies-to fields for it.
The condition select no longer waits for a column; a missing column is flagged
on Save. The agent wire, tool schema, prompt, structural guard, and shared
field validation cover the new kind.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A step now carries `from` and a value takes the highest step it reaches; the
rule's base color covers values below the lowest step. This matches threshold
lists elsewhere. The editor shows the base line first, then each step as ">= N".
The wire uses `steps[].from` and `base_color`. The old shape is dropped on load
by the structural guard; no migration.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- test each regex match from the start, so g and y no longer skip rows
- keep no trend rows for released cells or cells without comparison rules
- reject agent rules that the load check would drop on reload
- offer highlight rules only when the active result renders a grid

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Match regex rules with RE2 so no pattern can stall the grid; validation,
  the AI wire, the schema and the prompt describe the RE2 dialect
- Read zone-less timestamp bounds as UTC, in the engine and in validation
- Treat DECIMAL(p,s) columns as numeric
- Time flashes from the result capture, so late-mounted cells do not replay
- Close drawer sessions on the view toggle, the mode change and cell delete
- Signal a user edit on highlight save and clear for the AI stale gate
- Cover persistence, import, buildAppliedCells and multi-statement capture
- Route chart draft edits through one updater instead of an effect
- Drop the unused inline variant and props from SearchableSelect
- Add Given/When/Then markers to the new tests

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Keeps the query key in queryKey.ts and pins capitalize: false there, so
saved grid layouts keep their hash whatever the keyword-casing setting is.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Key the trend baseline by the effective SQL, never baseline a truncated
snapshot, and bring a discarded run back to the result it replaced.
Keep a released statement's baseline for its rehydrate, time a restored
result from its save, and start over when a run lands on a released cell.

Make the sliding-out drawer inert, close it when the assistant changes the
rules, and control the threshold and step inputs so Save flags bad values.
Accept only ISO timestamps, compare instants at full precision, clamp a
gradient with one automatic bound at its fixed end, and compare array
cells by content. Offer per column kind only the conditions that can match.

Load re2js on demand, evaluate rows on first lookup, paint the flash as a
layer over a pinned cell's opaque base, and keep the flash delay per result.
Escape angle brackets in the prompt JSON so rules copy back unchanged.
Drop the test-only parameters, the alpha field and the unused helpers.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Highlight engine
- Compare fractional DECIMAL past double precision as scaled bigints;
  =, >, > previous and changedBy now see a change at the 18th decimal
- Report an unmatched quote on a bound instead of saving a rule that
  never matches; one quotes helper for the evaluator and the validator

Drawers
- Kebab entries only open a drawer; the spotlight gear toggles, so a
  second menu pick keeps the open draft
- A save that waits on the regex engine is dropped after a dismiss,
  shows a spinner, and saves the draft as edited during the wait
- Chart settings OPEN telemetry fires on a real open only; the gear
  close reports a cancel
- Overlay slot is the first wrapper child, so grid layout grows the
  editor again

Agent tools
- Validate wire rules against the columns the cell has shown; a cell
  whose SQL the request rewrites is checked loosely
- A /…/flags pattern saves as plain RE2 and the result notes how it
  reads, pointing at (?i)
- Schema and prompt: high-scale numbers as strings, no regex flags

Trend store
- A discarded run restores the prior result with its landing time and
  keeps its statement keys until the run settles, so nothing reflashes

Tests
- e2e: kebab keeps highlight and chart drafts; toast text updated

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s, snapshot columns)

- changedBy compares as decimals so a change of exactly the threshold matches
- agent tools check rules against the snapshot columns of a released cell
- duplicate-key warning follows the draft identity in the drawer
- Enter on an empty picker search keeps the selection
- drawers take focus on open and return it to the opener on close
- one useSettingsDrawerSession hook for chart and highlight drawers

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@emrberk

emrberk commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Review: PR #613 "feat: show rule-based trends in ResultGrid"

Reviewed at level 3. Head 34293025, base 2a88dd23.

Issues

ID Issue name Category Severity Location Net impact Evidence Description Steps to reproduce Suggested fix
#1 Custom interval input fights menu focus Accessibility & UX Moderate in-diff All custom-interval users; keyboard users fully blocked Browser (Playwright) at 34293025, all 3 interval menus; input absent at 2a88dd23 CustomIntervalInput sits inside the Radix radio group, but it is not a menu item. Radix moves focus only between items and blocks Tab, so keyboard users cannot focus the input. Radix also focuses an item on pointer move. If the pointer passes over an item while the user types, focus leaves the input, the next keys go to typeahead, and the typed characters are lost. • Open "Notebook auto-refresh" with Enter.
• Press ArrowDown, End, Tab and Shift+Tab. Focus never reaches the input.
• In a cell interval menu, click the input and type "2".
• Move the pointer over "5s", then type "0s". The input shows "2", and focus is on "5s".
Add a "Custom…" item whose onSelect calls event.preventDefault() and focuses the input. While the input has focus, call preventDefault() in the items' onPointerMove, so Radix skips the item focus.
#2 Drawer loses focus on maximize Accessibility & UX Moderate in-diff Drawer users who maximize or restore; focus lost every time Browser focus log at 34293025, both directions; at 2a88dd23 maximize closed the drawer The drawer stays open across maximize and restore. After the remount, the overlay container in Cell.tsx is null on the first render, so CellOverlayPortal renders the drawer inline. The focus effect (deps [open, isDrawer]) focuses the inline title. Then the content moves into the portal, the title is replaced, and focus drops to body. The stored opener (the old Maximize button) is gone, so focus also does not return on close. This contradicts the PR claim "Focus returns to the opener after the slide-out ends". • Open a grid cell's More actions → Highlight rules.
• Click Maximize. The drawer stays open. document.activeElement is body.
• Press Escape. Focus stays on body.
• The Restore direction behaves the same.
Render null in CellOverlayPortal until the container exists. Or add the container to the focus effect deps.
#3 aria-modal without focus containment Accessibility & UX Moderate in-diff Screen reader users of both settings drawers Browser Tab sequence at 34293025; 2a88dd23 had no aria-modal In drawer mode, SettingsDrawerShell.tsx:300-301 sets role="dialog" and aria-modal="true". No focus trap exists, and no sibling is inert. Tab leaves the drawer after "Save" and reaches the cell header and the Monaco editor. Screen readers that honor aria-modal hide content that the user can still reach and use. • Open Highlight rules on a grid cell.
• Press Tab 7 times. Focus goes to "Name cell" outside the drawer, and the drawer stays open.
Remove aria-modal, because the cell header stays usable by design. If the drawer must be modal, add a trapped focus scope and make the rest of the cell inert.
#4 Fixed poll stalls after clock step Async, timers & cancellation Moderate in-diff Fixed-interval cells; rare clock steps; stall = step size Scratch vitest, fake timers: 61 s gap at 34293025, no gap at 2a88dd23 runFixedIntervalPollLoop.ts:15-20 measures elapsed time with Date.now() and does not cap the sleep. A clock step back of S during a fetch makes the next sleep interval + S. The cell stops refreshing with no error, and the label still shows the chosen rate. Base slept a fixed interval and timed fetches with performance.now(). runPollLoop already guards this case for its first wait. A tab switch, a scroll, or a manual refresh ends the stall. • Set a cell to "1s".
• Step the OS clock back by 60 s while a fetch runs (NTP step, manual change, VM restore).
• The cell does not refresh for about 61 s.
Measure elapsed time with performance.now(), or clamp remainingMs to intervalMs.
#5 One stale rule fails agent apply Persistence & migrations Moderate in-diff AI users with stale rules; whole apply_notebook_state rejected Scratch vitest through the dispatchTool harness at 34293025 applyNotebookState.ts:457-484 checks each copied-back highlight_config against the current columns. The rules are checked only when saved, and nothing flags a rule that goes stale later. A column type change keeps the SQL the same but breaks the rule. The prompt tells the agent to copy the config back, and that copy fails the whole atomic apply, so an edit to an unrelated cell is rejected. To pass, the agent must drop or change the user's rules. If it omits the config, the rules clear with no warning. • Save a contains "BTC" rule on SYMBOL column sym.
• Change sym to a numeric type with the same name, then run the cell.
• Ask the assistant to edit another cell.
• Result: VALIDATION_ERROR … Not for a numeric column. No cell changes.
Skip the column-fit check when the incoming config equals the stored one, or return the problem as a note instead of an error.
#6 Duplicate scan on every result Performance & rendering Moderate in-diff Every notebook grid; ≤2.5 ms median per result, 10k-row cap Scratch benchmark at 34293025; cost absent at 2a88dd23 ResultGridPanel always renders HighlightSettingsDrawer, also when it is closed. The drawer runs duplicateCountOf(draft.identityColumns) in a useMemo (HighlightSettingsDrawer.tsx:84-87), and the callback changes with each result. So every refresh scans all rows with JSON.stringify keys. The default identity is all text columns, so users without rules pay too. The cost is 1.5–2.5 ms median, up to about 8 ms p90, per result per grid at 10k rows. That is 35–60% of the JSON.parse time for the same payload, and it repeats on every tick down to 50 ms. • Run a 10k-row grid cell with a text column at "250ms".
• Do not open Highlight rules.
• Each result still runs duplicateRowCount over all rows.
Compute the count only while the drawer is open. For example, move the memo into the open drawer body.
#7 Engine fixed-interval path untested Test review & coverage Moderate in-diff Fixed-interval users if the wiring regresses to drift Mutation to base wiring at 34293025: 879/879 notebook tests stay green Only runFixedIntervalPollLoop.test.ts tests start-to-start cadence, and it tests the util alone. Engine tests mock fetches that resolve at once, so old and new timing look the same. The 1s e2e test in notebookHighlight.spec.js also passes with base timing. If the wiring regresses, fixed cells drift again: a 1s interval with a 400 ms query refreshes every 1.4 s. • Revert the fixed branch in cellRefreshEngine.ts to runAdaptivePollLoop with min = max = interval.
• Run yarn vitest run src/scenes/Editor/Notebook/. All tests pass.
Add one engine test: a fixed "1s" cell with a 400 ms fetch. Assert that the second fetch starts at about 1000 ms, not 1400 ms.
#8 Trend capture skip test is weak Test review & coverage Moderate in-diff Developers; skip removal goes undetected Mutation (skip disabled) at 34293025: 876/876 notebook tests stay green The test "skips a cell whose result and rules did not change" (resultTrendCapture.test.ts:111) asserts only that store.get() returns the same entry. store.capture() already returns the existing entry for the same result, so the test passes without the skip. The skip runs on every cells write, including each keystroke. • Change if (unchanged) continue to if (unchanged && false) continue in resultTrendCapture.ts.
• Run the notebook tests. All pass.
Spy on store.capture and store.retainStatements. Assert that an unchanged cell calls neither.
#9 Shimmer columns 14px narrower Styling & theming Minor in-diff Cells with ▲/▼ rules scrolled back in; one-time shift Scratch store test + static proof at 34293025; no glyph slot at 2a88dd23 The live grid adds DIRECTION_GLYPH_WIDTH (14px) to each ▲/▼ column. GridShimmer.displayColumnsFor does not add it, although its comment says the swap "shifts nothing". The baseline survives release and rehydrate, so the glyph slot exists on the first render after the swap. Columns that the user resized are not affected. • Add a > previous rule and let the cell refresh at least twice.
• Scroll the cell far off screen and back.
• At the swap, each ▲/▼ column widens by 14px, and the columns to its right shift.
Add the glyph width in displayColumnsFor for direction columns. Or reserve the slot from the rules in both places.
#10 Validation constants duplicated Code structure & types Moderate in-diff Developers; two sources of truth for rule ops N/A — static PREVIOUS_OPS is identical in isHighlightConfig.ts:11 and highlightConfigWire.ts:80. isScalar is also defined in both files (:24, :106). VALUE_OPS has the same name in both files but different contents (5 ops against 9). A new op needs two edits, and the shared name invites mistakes. N/A — static Move the shared sets and isScalar to one module under ResultGrid/highlight/. Give the two value-op sets distinct names.
#11 Dead default props in SearchableSelect Code structure & types Moderate in-diff Developers; optional props that every caller passes N/A — static, grep of callers SearchableSelect/index.tsx:183-188 defaults placeholder, searchPlaceholder, emptyLabel, noMatchLabel and dataHookBase. Both callers (IdentitySection.tsx, RuleRow.tsx) pass all five. The project rule makes a prop optional only when some callers omit it. N/A — static Make the props required and remove the defaults.
#12 AutoRefreshInterval type too wide Code structure & types Minor in-diff Developers; type accepts values that runtime rejects N/A — static, scratch tsc check `${number}${"ms" | "s" | "m"}` (store/notebook.ts:25) accepts "1.5s", "-5s" and "0ms". intervalMsOf rejects all of them at runtime. N/A — static Use a branded string type that only parseAutoRefreshInterval and isAutoRefresh produce.
#13 Hint text wrong for format errors Accessibility & UX Minor in-diff Users who type "1h", "1.5s" or text N/A — static Every parse failure shows "Should be between 50ms and 60m" (CustomIntervalInput.tsx:71). "1h" is equal to the 60m limit but fails because h is not a unit. "1.5s" and "abc" are format errors, not range errors. • Type "1h" in the custom interval input.
• Press Enter. The range hint shows.
Show a format hint, for example "Use a whole number with ms, s or m, from 50ms to 60m".

Adjacent findings (not blocking — file as issues)

None proved.

Summary

Verdict

Approve with comments.

Please address these Moderate items before merge:

Gates

  • Correctness gate: pass. There is no admitted Critical finding. yarn typecheck, yarn lint, yarn test:unit (2524 tests) and yarn build all pass.
  • Test gate: pass. There are 2 admitted coverage findings, both Moderate (clean web console #7, run web console #8). DOM and component gaps (useHeldFlag, drawer focus) are accepted by project policy.

Distribution

  • Severity: 0 Critical, 10 Moderate, 3 Minor.
  • In-diff / out-of-diff: 13 / 0.
    • The cross-context pass covered every out-of-diff consumer: the main-console ResultGridAdapter and ResultChart, all Checkbox consumers, the Metrics ColorPalette, importTabs, headless agent runs, the snapshot and hydration paths, and the MCP tool schemas.
    • I found no broken consumer contract.

Regressions and tradeoffs

  • Chart animation: line, area, step and scatter series now never animate. This also applies to the main-console Result chart, which the PR description does not mention. The 500 ms animation gate has no effect on these types.
  • Row hover: hover in both grids now blends over gridRow instead of surfaceInset. THEMING.md documents this.
  • Refresh button: the button stays busy for 1 s after each fetch. It is always busy when the idle gap between fixed-interval fetches is shorter than 1 s. This also occurs above 1s with slow queries, for example 5s with a 4.5 s query.
  • Fixed-interval cadence: fixed intervals now run start-to-start. A query slower than its interval runs back-to-back with no idle gap. With the 50 ms minimum, one cell can keep the server busy all the time.
  • Row caps: NOTEBOOK_ROW_CAP and MAX_INDEXED_ROWS are both 10,000, and no code links them. If the fetch cap goes up, rows past 10,000 show as new on every refresh.

PR title

The title follows Conventional Commits. It names an internal component and omits the custom refresh intervals.

Suggestion: feat: highlight rules and custom auto-refresh intervals for notebook grids.

@emrberk
emrberk marked this pull request as ready for review September 30, 2026 10:01
@emrberk
emrberk merged commit 610f204 into main Sep 30, 2026
5 checks passed
@emrberk
emrberk deleted the feat/grid-highlight-rules branch September 30, 2026 16:18
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.

2 participants