Mobile queries benchmark - #51
Conversation
…to beast/mobile-queries-benchmark
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
REQUEST_CHANGES: The mobile suite does not benchmark the current app's history-query plan, and it can report a successful run without executing any benchmark query.
-
[P1] Mirror the query shape the mobile app actually sends (
src/lib/graphql-benchmark/mobile-queries.ts:163,src/lib/graphql-benchmark/mobile-registry.ts:97). The newAccountEvents*/scheduled documents use oneaccount_eventselection withaccount_id: {_in: $accounts}, order only by timestamp/id, and the registry always supplies one account. Currentquantus-apps/main@11e035a3deliberately emits one aliased_eqselection per wallet account and includesaccount_idplus the direction column inorder_by; its builder explains that_indoes not use the intended composite index. These shapes produce materially different database plans, and the current suite also misses multi-account fan-out, so its timings cannot characterize the mobile app or guard its performance. Please generate the same alias-per-account documents/order and exercise representative account counts, ideally with a drift check against the SDK query builders. -
[P1] Fail when mobile bootstrap cannot reach or query the endpoint (
src/lib/graphql-benchmark/mobile-bootstrap.ts:14,scripts/graphql-bench.ts:58). Every bootstrap exception is discarded, while skipped rows are excluded fromhasFailure. Reproduction:GRAPHQL_BENCH_URL=http://127.0.0.1:1/graphql npx tsx scripts/graphql-bench.ts --suite mobile --samples 1prints an empty context, marks every operationSKIPPED, and exits0. This makes an unavailable endpoint or incompatible schema look like a successful benchmark, and the new per-query timeout does not cover bootstrap. Preserve optional-data skips, but propagate/log bootstrap request failures and return nonzero when no meaningful suite ran; apply the caller's abort/timeout during bootstrap as well.
Validation at exact head 4274f298ec2b9054186b707228e7fd54a2d1bedd:
npm run build(Vite +tsc): passed; Bun was unavailable locally, so dependencies were installed transiently with npm.git diff --checkand Prettier over all changed files: passed.- Live
--suite mobile --samples 1against the default endpoint: completed and confirmed the documents are schema-valid; four data-dependent operations were skipped. - Invalid-endpoint reproduction above: exited
0with all operations skipped. - ESLint over changed files: failed on import/export sorting in
index.ts,mobile-bootstrap.ts, andmobile-registry.ts. - GitHub's exact-head build and dependency-cooldown checks are green. Jest could not collect tests in the local npm environment because the repository's TS transform setup was not applied.
The two correctness issues above are blocking.
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
APPROVE: The two blocking findings from the review at 4274f298 are resolved at dade877e; no blocking findings remain.
- The history benchmarks now reproduce
quantus-apps/main@11e035a3's alias-per-account_eqquery shape, composite ordering, keyset pagination, and multi-account fan-out. - Mobile bootstrap requests now use the benchmark timeout/abort signal, expose request failures, and make an unavailable endpoint fail the CLI instead of succeeding with an entirely skipped suite.
- The latest commit fixes the TypeScript error in the new failure-helper test; both the local production build and GitHub's exact-head Build and Test check pass.
Validation:
npm run build,npx tsc --noEmit --pretty false,git diff --check, and changed-file Prettier: passed.- Live
--suite mobile --samples 1againstsqm.quantus.com: completed with all runnable queries successful, including the new 2- and 8-account shapes. - Invalid-endpoint reproduction: reported the bootstrap failure, skipped dependent operations, and exited
1. - The repository's Jest configuration still cannot collect TypeScript tests because no TypeScript transform is configured. Changed-file ESLint also retains one non-blocking
simple-import-sort/exportserror insrc/lib/graphql-benchmark/index.ts.
Added benchmark for mobile queries