Skip to content

QueryDto has 16 representable combinations for 3 real states #74

Description

@Artmann

What's wrong

QueryDto represents query state as four independent fields — error: NullOr(String), finishedAt: NullOr(Number), result: NullOr(QueryResultDto), truncated: Boolean (src/glue/api/schemas.ts:255-264). That is 16 representable combinations for 3 real states.

Because the type does not say which is which, the same three-step ladder (finishedAt === null → running; else error !== null → failed; else succeeded) is re-derived in a dozen places:

App.tsx:218, hooks/queries.ts:159,178,182, StatusBar.tsx:38-65 (twice), use-worksheet-messages.ts:31-52, QueryResultContent.tsx:21-42, ResultsPane.tsx:143, mutations.ts:28, use-start-query.ts:37, main/queries/reconcile-queries.ts:17-18, query-runner.ts:139,157,406.

"Canceled" is a first-class state that exists nowhere in the type. It is error === canceledQueryMessage (glue/queries.ts:3, the literal string 'Query canceled.'), compared by string equality at QueryResultContent.tsx:56, and it has genuinely distinct behaviour: a different component branch, span status ok instead of error (tracing/query-traces.ts:32-38), and a suppressed toast (hooks/queries.ts:203).

A symptom worth quoting: QueryResultContent.tsx:30 reads if (query?.error !== undefined && query?.error !== null) — a defensive double-check on a field the contract types as string | null.

Ship this part first (independent, near-zero risk)

The top-level truncated field is dead and duplicated. It is written only as a copy of the nested value (query-runner.ts:410), and every reader uses query.result.truncated instead (use-worksheet-messages.ts:51, StatusBar.tsx:61, ResultsPane.tsx:33). About 10 fixture files have to remember to set a field nothing reads.

Deleting it is a self-contained change and is a prerequisite for the query-list projection fix (see the "query list loads full result sets into memory" issue).

The larger change

Schema.Union on status: 'running' | 'succeeded' | 'failed' | 'canceled', each variant carrying only the fields legal for it. The SQL columns do not changestatus is derived in transformQueryRow (query-runner.ts:396-412), the single server-side mapping.

Scope: schemas.ts, 3 producers, ~7 consumers, ~11 test-fixture files. No DB migration.

Risks

  1. mutations.ts:46-49 mutates a TanStack DB draft field-by-field. Under a union this must become a whole-row replace — prototype this first. (Note that the cancel-flow fix in the "cancel is represented twice" issue removes this call site's reason to exist, so doing that one first makes this simpler.)
  2. transformQueryRow must stay total for legacy rows, or it re-creates the query-runner.test.ts:258-296 regression where one bad row 400s the entire history list.
  3. Encoding now enforces the correlation, so previously-tolerated contradictions become hard decode failures.

Validation

Existing state coverage is good: query-runner.test.ts:87-95,144-145,188-192,258-296,310-324, use-worksheet-messages.test.ts:62-69,108,116,137, queries.test.tsx:68,83,119.

Add: decode tests rejecting running+result and succeeded+error; a transformQueryRow case per legacy row shape; a list-encode test containing one legacy row.


Found in a codebase-wide simplification audit (F-S01-a). Raised independently by three separate review lanes. Confidence: medium-high on the problem, medium on the full union clearing its migration cost — hence the "ship the deletion first" split.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions