Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion api/src/damnit_api/graphql/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,18 @@ async def metadata(
"timestamp": snapshot["timestamp"] * 1000, # ms for JS
} # pyright: ignore[reportReturnType]

# Nullable, because a preview asks for many runs in one request, aliasing
# this field once per run. A non-null field that raises propagates the null
# up to the root, so a single unreadable run would take every other run in
# the request down with it.
@strawberry.field(permission_classes=PROPOSAL_PERMISSIONS)
async def extracted_data(
self,
info: Info,
database: DatabaseInput,
run: int,
variable: str,
) -> JSON: # FIX: # pyright: ignore[reportInvalidTypeForm]
) -> JSON | None: # FIX: # pyright: ignore[reportInvalidTypeForm]
await _ensure_damnit_path(info, database.proposal)
# TODO: Convert to Strawberry type
# and make it analogous to DamitVariable; e.g. `data`
Expand Down
45 changes: 44 additions & 1 deletion frontend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ packages/ui/src/
components/ presentational only: no store, auth, or Apollo
graphql/ Apollo client, shared documents, operation names
lib/ utils/ styles/ shared leaves
data/ temporary: server state in Redux, being migrated to Apollo
data/ the table slice and the proposal queries: the server state
still held outside Apollo
```

Imports run one way: `components / lib / utils / graphql` <- `features` <-
Expand All @@ -52,6 +53,48 @@ Imports outside a file's own folder use the `#src/*` subpath import
(`#src/utils/array`), never `../`. Same-folder imports stay relative (`./`).
That keeps a file's imports stable when the tree moves, and it is enforced.

### Data flow

Every GraphQL fetch goes through an Apollo hook. What a screen renders _from_
depends on whether the schema lets Apollo key the data:

- **Table rows and summary plots** flow from hooks into the `tableData` Redux
slice, which owns the merge. `DamnitRun` has no id: the API models the run
number as a known variable alongside the real ones, so there is nothing for
Apollo to normalize and the merge cannot move into the cache yet. `TablePageLoader`
renders one instance per wanted page, since a component can own only one
watched query, and each dispatches its own rows.
- **Preview plots** render straight from the Apollo cache, with no slice.
`extracted_data` fetches one run per call and has no batch field, so
`usePreviewPlotData` builds a document that aliases the field per run
(`r142: extracted_data(run: 142, ...)`), chunked by run value. One
`PreviewChunkLoader` per chunk fetches cache-and-network, while the parent
watches the whole aliased document cache-only with `returnPartialData`, so the
plot fills in as chunks land. Aliases are erased in the store, so plots that
overlap on a run read the one entry; each chunk still revalidates it.

Two cache behaviours are worth knowing before you touch a runs document:

- **A directive is part of the store key.** The lightweight and deferred queries
send identical `runs(...)` arguments, so the deferred write looks certain to
clobber the lightweight one. It does not: the cache holds
`runs({...})@lightweight` and `runs({...})` as two fields. That separation only
holds because `@lightweight` is in the document.
- **Column subsets replace, they do not merge.** Because `DamnitRun` is
unnormalized the runs array is stored inline, and every write replaces it
whole, so two documents asking for different `names` at the same page evict
each other rather than sharing. This is why summary plots fetch `no-cache`:
the slice is the only thing that renders their rows, so a cached copy is never
read, and caching one would only evict the table's. Do not paper over it with
typePolicies against the unkeyed schema; the real fix is a run key on the
backend.

Both are symptoms of the same missing key. Once the backend gives `DamnitRun`
one and adds a batch preview field, runs normalize in the cache, pagination
becomes one query plus `fetchMore`, the grid and summary plots render from the
cache, and the `tableData` slice goes away. Summary plots go back to
cache-and-network with it.

## Installation

Clone the project and run the following on the root folder:
Expand Down
3 changes: 2 additions & 1 deletion frontend/apps/demo/src/mocks/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ const source: MockDataSource = {
}

const gqlHandlers = [
api.operation(async ({ operationName, variables }) => {
api.operation(async ({ operationName, query, variables }) => {
const resolution = await resolveOperation(operationName, {
query,
variables: variables as Record<string, unknown>,
source,
})
Expand Down
7 changes: 5 additions & 2 deletions frontend/e2e/mocks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@ export async function mockApi(
})

await page.route('**/graphql', async (route) => {
const { operationName, variables } = (route.request().postDataJSON() ??
{}) as {
const { operationName, query, variables } = (route
.request()
.postDataJSON() ?? {}) as {
operationName: string
query: string
variables?: Record<string, unknown>
}

Expand All @@ -182,6 +184,7 @@ export async function mockApi(

try {
const resolution = await resolveOperation(operationName, {
query,
variables: variables ?? {},
source,
})
Expand Down
1 change: 1 addition & 0 deletions frontend/e2e/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"@types/node": "catalog:types",
"eslint": "catalog:linting",
"globals": "catalog:linting",
"graphql": "catalog:graphql",
"monaco-editor": "catalog:components",
"prettier": "catalog:linting",
"typescript": "catalog:tooling"
Expand Down
6 changes: 3 additions & 3 deletions frontend/e2e/support/plots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ export async function selectCells(page: Page, cells: Cell[]) {
)
}

// Right-click a cell and pick "Plot: data". Right-clicking a cell that is not
// Right-click a cell and pick "Plot: preview". Right-clicking a cell that is not
// already selected resets the selection to it, so build any multi-run range
// with selectCells first, then right-click one of the selected cells.
export async function openDataPlot(page: Page, cell: Cell) {
export async function openPreviewPlot(page: Page, cell: Cell) {
await rightClickCell(page, cell)
await contextMenu(page).getByText('Plot: data').click()
await contextMenu(page).getByText('Plot: preview').click()
}

// The plot dialog ("Display Plot" -> "Plot Settings" modal). Returns the dialog
Expand Down
16 changes: 8 additions & 8 deletions frontend/e2e/tests/app/live-updates/live-updates.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ test('a finished run appears as a new row', async ({ page, api, example }) => {
const grid = page.locator('[role="grid"]')
const seedRuns = XPCS.meta.runs.length

// Wait for the seed's data to land before pushing, so getTable.fulfilled cannot
// revert the new run afterwards. Gate on a populated cell, not aria-rowcount:
// the metadata query fills runs (and the row count) with no cell data, so the
// row count can reach seedRuns + 1 before getTable.fulfilled has landed.
// Wait for the seed's data to land before pushing, so the page's own rows
// cannot revert the new run afterwards. Gate on a populated cell, not
// aria-rowcount: the metadata query fills runs (and the row count) with no
// cell data, so the row count can reach seedRuns + 1 before the rows land.
await expect(
cell(page, { col: columnOf('n_trains'), row: 0 })
).not.toBeEmpty()
Expand Down Expand Up @@ -66,9 +66,9 @@ test("an existing run's value updates live", async ({ page, api, example }) => {
await openProposal(page, example)
const trains = cell(page, { col: columnOf('n_trains'), row: 0 })

// Wait for the seed value to land before pushing, so getTable.fulfilled cannot
// revert the update afterwards. The push then sets a value the seed never had,
// so the cell text flipping to it proves the update rendered.
// Wait for the seed value to land before pushing, so the page's own rows
// cannot revert the update afterwards. The push then sets a value the seed
// never had, so the cell text flipping to it proves the update rendered.
await expect(trains).not.toBeEmpty()
const updated = '999999'
await expect(trains).not.toHaveText(updated)
Expand All @@ -95,7 +95,7 @@ test.describe('a deferred image resolves after its run finished', () => {
const preview = card.locator('img')

// Wait for the seed data to land (run 1's n_trains is populated) before
// pushing, so getTable.fulfilled cannot revert the image afterwards.
// pushing, so the page's own rows cannot revert the image afterwards.
await expect(
cell(page, { col: columnOf('n_trains'), row: 0 })
).not.toBeEmpty()
Expand Down
22 changes: 12 additions & 10 deletions frontend/e2e/tests/app/plots/plot-dialog.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { previewRunFields } from '@damnit-frontend/shared/mocks'
import { test, expect } from '#fixtures'
import { openProposal, titleOf } from '#support/table'
import {
Expand Down Expand Up @@ -43,7 +44,7 @@ test('choosing a Y variable plots a summary against Run', async ({
await expect(plotFigure(page)).toBeVisible()
})

test('plotting data for a custom run set opens a data plot for those runs', async ({
test('plotting a preview for a custom run set opens a preview plot for those runs', async ({
page,
example,
}) => {
Expand All @@ -52,32 +53,33 @@ test('plotting data for a custom run set opens a data plot for those runs', asyn

// The "run 7-9" subtitle only shows first-last, so collect the runs actually
// fetched: the comma input is a discrete set, so exactly runs 7 and 9 should
// fire an ExtractedDataQuery, not the range 7..9.
// be asked for, not the range 7..9. A preview inlines its runs into the
// document rather than passing them as variables, so read them back from it.
const requestedRuns: number[] = []
page.on('request', (request) => {
if (!request.url().includes('/graphql')) {
return
}
const { operationName, variables } = (request.postDataJSON() ?? {}) as {
const { operationName, query } = (request.postDataJSON() ?? {}) as {
operationName?: string
variables?: { run?: number }
query?: string
}
if (operationName === 'ExtractedDataQuery') {
requestedRuns.push(variables?.run as number)
if (operationName === 'PreviewDataQuery' && query) {
requestedRuns.push(...previewRunFields(query).map((field) => field.run))
}
})

// Switching to "Plot data" reveals the Variable combobox and the custom-runs
// input; the dialog is the only path to plotting an arbitrary run set.
await dialog.getByText('Plot data').click()
// Switching to "Plot preview" reveals the Variable combobox and the
// custom-runs input; the dialog is the only path to an arbitrary run set.
await dialog.getByText('Plot preview').click()
await chooseVariable(dialog, {
label: 'Variable',
title: titleOf('xgm_intensity'),
})
await dialog.getByPlaceholder('e.g. 1,2,3,6-20,22').fill('7,9')
await submitPlot(dialog)

const tab = plotTab(page, `Data: ${titleOf('xgm_intensity')}`)
const tab = plotTab(page, `Preview: ${titleOf('xgm_intensity')}`)
await expect(tab).toBeVisible()
await expect(tab).toContainText('run 7-9')
await expect(plotFigure(page)).toBeVisible()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { test, expect } from '#fixtures'
import { XPCS } from '#examples/xpcs'
import { columnOf, openProposal, titleOf } from '#support/table'
import {
openDataPlot,
openPreviewPlot,
PLOT_VIEWPORT,
plotFigure,
plotTab,
Expand All @@ -11,21 +11,21 @@ import {

test.use({ viewport: PLOT_VIEWPORT })

// Variables chosen for the dtype of their *extracted* data, which differs from
// Variables chosen for the dtype of their *preview* data, which differs from
// the scalar shown in the table cell. One per render branch:
const SCATTER_VAR = 'xgm_intensity' // 1-D array -> Plotly scatter
const SCALAR_VAR = 'n_trains' // scalar number -> "unable to display" notice
const IMAGE_VAR = 'xpcs_g2_plot' // png -> plain <img>

test('right-clicking a cell and choosing "Plot: data" plots its extracted data', async ({
test('right-clicking a cell and choosing "Plot: preview" plots its preview data', async ({
page,
example,
}) => {
await openProposal(page, example)

await openDataPlot(page, { col: columnOf(SCATTER_VAR), row: 0 })
await openPreviewPlot(page, { col: columnOf(SCATTER_VAR), row: 0 })

await expect(plotTab(page, `Data: ${titleOf(SCATTER_VAR)}`)).toBeVisible()
await expect(plotTab(page, `Preview: ${titleOf(SCATTER_VAR)}`)).toBeVisible()
await expect(plotFigure(page)).toBeVisible()
})

Expand All @@ -41,9 +41,9 @@ test('selecting cells from two runs plots both in one figure', async ({
{ col, row: 0 },
{ col, row: 1 },
])
await openDataPlot(page, { col, row: 1 })
await openPreviewPlot(page, { col, row: 1 })

const tab = plotTab(page, `Data: ${titleOf(SCATTER_VAR)}`)
const tab = plotTab(page, `Preview: ${titleOf(SCATTER_VAR)}`)
await expect(tab).toBeVisible()
await expect(tab).toContainText('run 1-2')
await expect(plotFigure(page)).toHaveCount(1)
Expand All @@ -55,12 +55,12 @@ test('right-clicking a scalar cell shows the unable-to-display notice', async ({
}) => {
await openProposal(page, example)

// The extracted value equals the scalar the table already shows for run 1.
// The preview value equals the scalar the table already shows for run 1.
const scalarValue = XPCS.data[0].variables[SCALAR_VAR].value

await openDataPlot(page, { col: columnOf(SCALAR_VAR), row: 0 })
await openPreviewPlot(page, { col: columnOf(SCALAR_VAR), row: 0 })

await expect(plotTab(page, `Data: ${titleOf(SCALAR_VAR)}`)).toBeVisible()
await expect(plotTab(page, `Preview: ${titleOf(SCALAR_VAR)}`)).toBeVisible()
await expect(page.getByText('Unable to display the plot')).toBeVisible()
await expect(page.getByText(/scalar/)).toContainText(String(scalarValue))
})
Expand All @@ -71,9 +71,9 @@ test('right-clicking a png cell renders the image instead of a figure', async ({
}) => {
await openProposal(page, example)

await openDataPlot(page, { col: columnOf(IMAGE_VAR), row: 0 })
await openPreviewPlot(page, { col: columnOf(IMAGE_VAR), row: 0 })

await expect(plotTab(page, `Data: ${titleOf(IMAGE_VAR)}`)).toBeVisible()
await expect(plotTab(page, `Preview: ${titleOf(IMAGE_VAR)}`)).toBeVisible()
await expect(page.locator('img[src^="data:image/png"]')).toBeVisible()
await expect(plotFigure(page)).toHaveCount(0)
})
2 changes: 1 addition & 1 deletion frontend/e2e/tests/app/table/image-preview.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ test('right-clicking an image cell dismisses the preview and opens the context m
// Right-click the same cell: the preview clears and the plot menu opens.
await rightClickCell(page, IMAGE_CELL)
await expect(preview).toBeHidden()
await expect(page.getByText('Plot: data')).toBeVisible()
await expect(page.getByText('Plot: preview')).toBeVisible()
})

test('hovering a cell with no supported tooltip shows nothing', async ({
Expand Down
4 changes: 4 additions & 0 deletions frontend/packages/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
"lint:prettier": "prettier --check --cache --ignore-path ../../.prettierignore .",
"lint:tsc": "tsc -b --noEmit"
},
"peerDependencies": {
"graphql": "catalog:graphql"
},
"devDependencies": {
"@damnit-frontend/config": "workspace:*",
"eslint": "catalog:linting",
"graphql": "catalog:graphql",
"prettier": "catalog:linting",
"typescript": "catalog:tooling"
}
Expand Down
1 change: 1 addition & 0 deletions frontend/packages/shared/src/mocks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export {
} from './shape'
export {
MockDataNotFound,
previewRunFields,
resolveOperation,
type MockDataSource,
} from './resolve'
Expand Down
Loading