fix(query): wire driver truncation signal + user-configurable maxRows (#499) - #507
Merged
Merged
Conversation
…#499) ## The bug The row-cap "Showing first 10,000 rows" banner was unreachable dead code. Both database drivers (PG + Neo4j) capped results at `config.rowLimit = 5000` BEFORE the API route could see them, so the route's `rawData.length > MAX_ROWS` comparison (against 10,000) was never true. `meta.truncated` was never set, `widgetQuery.data.truncated` was never true, and the banner in card-container.tsx:569 never rendered. Both drivers ALREADY computed `isTruncated` internally and called `callbacks.setStatus(QueryStatus.COMPLETE_TRUNCATED)` — but the executor's callback object at query-executor.ts:127-135 didn't implement `setStatus`, so the signal was silently dropped. The glue layer between driver and route was missing. ## The fix ### 1. Wire setStatus through query-executor query-executor.ts now provides a setStatus handler that captures the `COMPLETE_TRUNCATED` enum into a local `truncated` boolean. The promise resolves with `{ data, fields, truncated, rowLimit }`. The route forwards `truncated` and `rowLimit` into response meta, and removes the dead `rawData.length > MAX_ROWS` check + the `MAX_ROWS = 10_000` constant entirely. Exported `QueryStatus` from @neoboard/connection since the executor now depends on it (previously only internal to the connection package). ### 2. User-configurable maxRows per connection Added `maxRows` as an optional advanced setting on each connection. When unset, defaults to `DEFAULT_MAX_ROWS = 5000` (backwards-compatible with today's behavior). Creators can raise or lower the cap per connection via Advanced Settings > "Max Rows per Query" in the connection form. - `lib/shared/schemas.ts` — zod field, validated 100..100_000 - `lib/query/query-executor.ts` — `ConnectionCredentials.maxRows`, spread `rowLimit: effectiveRowLimit` onto driver config, capture in `DEFAULT_MAX_ROWS` export - `app/(dashboard)/connections/page.tsx` — new input in both create + edit dialogs, help text explaining the memory tradeoff - `hooks/use-connections.ts` — type extension - `lib/shared/parse-utils.ts` — form state round-trip ### 3. Dynamic banner text `card-container.tsx` now renders `"Showing first {rowLimit} rows"` using the dynamic `rowLimit` field forwarded from meta. Previously hardcoded "10,000" regardless of the actual cap. `use-widget-query.ts` `QueryResult` interface picks up a new optional `rowLimit?: number` field so TypeScript flows the value through. ## Verification - [x] Route unit tests: 18/18 passing — rewrote 4 tests to mock the new { truncated, rowLimit } return shape; added a test for the per-connection override path. - [x] Card container unit tests: 13/13 passing (was 11). Added a new test asserting the banner reflects a custom per-connection rowLimit (25,000 in the mock), flipped the existing "Showing first 10,000" assertion to "Showing first 5,000" to match the default. - [x] parse-utils tests: 15/15 passing — added maxRows to the round-trip fixtures. - [x] E2E query-safety spec: 8/8 passing in 60s. Tests 3 and 4 now assert meta.truncated === true AND the banner renders with the dynamic text. Added test 4b: create a PG connection with `maxRows: 1000`, run a query returning 5000 rows, verify the driver honors the override (body.data.data.length === 1000, meta.rowLimit === 1000, UI banner shows "Showing first 1,000 rows"). - [x] Build clean, lint clean (only pre-existing errors in cli/ unrelated files). ## Backwards compatibility - `maxRows` is optional. Existing connections (seeded conn-pg-001 / conn-neo4j-001 / any user-created) continue to use DEFAULT_MAX_ROWS=5000 with no config change. - The API response shape is additive: new `meta.rowLimit` is always present, `meta.truncated` appears only when the driver signals it (same as before). Existing clients that don't read these fields are unaffected. - No DB migration needed — `maxRows` lives in the encrypted connection config JSON alongside the existing advanced settings. Closes #499 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughDriver-level row-cap signaling was implemented and propagated through the executor and API route. Per-connection Changes
Sequence DiagramsequenceDiagram
participant Client
participant Route as API Route
participant Executor as Query Executor
participant Driver as Connection Driver
Client->>Route: POST /api/query (sql + credentials)
Route->>Executor: executeQuery(sql, credentials)
Executor->>Executor: effectiveRowLimit = credentials.maxRows ?? DEFAULT_MAX_ROWS
Executor->>Driver: runQuery(config with rowLimit)
Driver->>Driver: execute and apply rowLimit, detect truncation (setStatus)
Driver->>Executor: onSuccess(data, fields, truncated?)
Executor->>Route: resolve {data, fields, truncated, rowLimit}
Route->>Client: respond {data, meta:{truncated, rowLimit}}
Client->>Client: UI renders banner using meta.rowLimit when truncated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/app/(dashboard)/connections/page.tsx (1)
325-344:⚠️ Potential issue | 🟠 MajorClearing
maxRowsin edit won't remove an existing override.If the user deletes the value,
parseOptionalInt("")becomesundefined, and that key gets dropped from the PATCH payload. Per the partial-update contract in the comment above, the backend then keeps the old storedmaxRows, so this new override is effectively sticky once set. Please add explicit clear semantics here (for example, send a nullable/reset value or expose an explicit “use default” control).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/app/`(dashboard)/connections/page.tsx around lines 325 - 344, The buildEditConfig function currently drops maxRows when the user clears the input (because parseOptionalInt("") -> undefined), causing the backend to retain the previous override; update buildEditConfig to send an explicit clear/reset marker instead of omitting the key — e.g. detect when editForm.maxRows is an empty string and set maxRows: null (or set a companion flag like maxRowsUseDefault: true) so the PATCH payload signals the server to remove the override; refer to buildEditConfig and editForm.maxRows and parseOptionalInt when making this change.
🧹 Nitpick comments (3)
app/src/lib/query/query-executor.ts (1)
17-17: Avoid hardcoded row-cap default drift.Use the shared connection default instead of literal
5000so app and connector defaults cannot diverge.♻️ Suggested change
-export const DEFAULT_MAX_ROWS = 5000; +export const DEFAULT_MAX_ROWS = DEFAULT_CONNECTION_CONFIG.rowLimit;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/lib/query/query-executor.ts` at line 17, Replace the hardcoded literal in DEFAULT_MAX_ROWS by referencing the shared connection default instead of 5000: update the constant DEFAULT_MAX_ROWS to import and use the project-wide connection max-rows default (e.g., ConnectionDefaults.MAX_ROWS or CONNECTION_DEFAULTS.maxRows) from the shared connection/config module so the app and connector share one source of truth; modify the export in query-executor.ts to reference that imported symbol and add the corresponding import statement.app/src/hooks/use-connections.ts (1)
85-85: Consider aligning create/test-inline config types withmaxRowstoo.
UpdateConnectionInputnow supportsmaxRows; mirroring this inCreateConnectionInputandTestInlineInputwould keep client API types consistent and reduce drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/hooks/use-connections.ts` at line 85, CreateConnectionInput and TestInlineInput need the same maxRows support as UpdateConnectionInput to keep the client API consistent: add an optional maxRows?: number property to both CreateConnectionInput and TestInlineInput type/interface declarations, and propagate that field through any creators/validators/serializers (e.g., functions building payloads for createConnection and testInline) so the value is sent/validated the same way UpdateConnectionInput handles it.app/src/app/api/query/__tests__/route.test.ts (1)
470-506: Assert the route passesmaxRowsintoexecuteQuery.This case currently hardcodes
mockExecuteQueryto returnrowLimit: 20000, so it would still pass if the route stopped forwardingdecryptJson(...).maxRowsto the executor. Add a call-argument assertion here to pin the actual route-to-executor wiring for#499.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/app/api/query/__tests__/route.test.ts` around lines 470 - 506, The test currently stubs mockExecuteQuery to return rowLimit:20000 but doesn't assert the route actually forwarded decryptJson(...).maxRows into the executor; update the test after calling POST(makeRequest(...)) to assert mockExecuteQuery was invoked with a payload containing maxRows: 20000 (e.g. expect(mockExecuteQuery).toHaveBeenCalledWith(expect.objectContaining({ maxRows: 20000 })) or inspect mockExecuteQuery.mock.calls[0][0].maxRows) so the route-to-executor wiring (POST handler -> decryptJson -> executeQuery) is pinned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/card-container.tsx`:
- Around line 573-574: The banner currently falls back to 0 when
widgetQuery.data.rowLimit is missing, producing misleading text; update the JSX
around the snippet that renders "Showing first {(widgetQuery.data.rowLimit ??
0).toLocaleString()} rows" to conditionally render a non-numeric fallback (e.g.,
show the number only when widgetQuery.data.rowLimit != null and otherwise render
"Showing limited rows" or "Showing first N rows" with a known default like
widgetQuery.defaultRowLimit), or use a string fallback such as
(widgetQuery.data.rowLimit != null ? widgetQuery.data.rowLimit.toLocaleString()
: "limited") so the message is not "0" when rowLimit is absent; adjust any
related text in the same component (card-container / the banner JSX)
accordingly.
---
Outside diff comments:
In `@app/src/app/`(dashboard)/connections/page.tsx:
- Around line 325-344: The buildEditConfig function currently drops maxRows when
the user clears the input (because parseOptionalInt("") -> undefined), causing
the backend to retain the previous override; update buildEditConfig to send an
explicit clear/reset marker instead of omitting the key — e.g. detect when
editForm.maxRows is an empty string and set maxRows: null (or set a companion
flag like maxRowsUseDefault: true) so the PATCH payload signals the server to
remove the override; refer to buildEditConfig and editForm.maxRows and
parseOptionalInt when making this change.
---
Nitpick comments:
In `@app/src/app/api/query/__tests__/route.test.ts`:
- Around line 470-506: The test currently stubs mockExecuteQuery to return
rowLimit:20000 but doesn't assert the route actually forwarded
decryptJson(...).maxRows into the executor; update the test after calling
POST(makeRequest(...)) to assert mockExecuteQuery was invoked with a payload
containing maxRows: 20000 (e.g.
expect(mockExecuteQuery).toHaveBeenCalledWith(expect.objectContaining({ maxRows:
20000 })) or inspect mockExecuteQuery.mock.calls[0][0].maxRows) so the
route-to-executor wiring (POST handler -> decryptJson -> executeQuery) is
pinned.
In `@app/src/hooks/use-connections.ts`:
- Line 85: CreateConnectionInput and TestInlineInput need the same maxRows
support as UpdateConnectionInput to keep the client API consistent: add an
optional maxRows?: number property to both CreateConnectionInput and
TestInlineInput type/interface declarations, and propagate that field through
any creators/validators/serializers (e.g., functions building payloads for
createConnection and testInline) so the value is sent/validated the same way
UpdateConnectionInput handles it.
In `@app/src/lib/query/query-executor.ts`:
- Line 17: Replace the hardcoded literal in DEFAULT_MAX_ROWS by referencing the
shared connection default instead of 5000: update the constant DEFAULT_MAX_ROWS
to import and use the project-wide connection max-rows default (e.g.,
ConnectionDefaults.MAX_ROWS or CONNECTION_DEFAULTS.maxRows) from the shared
connection/config module so the app and connector share one source of truth;
modify the export in query-executor.ts to reference that imported symbol and add
the corresponding import statement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36b36317-043b-4282-96e9-7bc65513a0b4
📒 Files selected for processing (13)
app/e2e/query-safety.spec.tsapp/src/app/(dashboard)/connections/page.tsxapp/src/app/api/query/__tests__/route.test.tsapp/src/app/api/query/route.tsapp/src/components/__tests__/card-container-states.test.tsxapp/src/components/card-container.tsxapp/src/hooks/use-connections.tsapp/src/hooks/use-widget-query.tsapp/src/lib/__tests__/shared/parse-utils.test.tsapp/src/lib/query/query-executor.tsapp/src/lib/shared/parse-utils.tsapp/src/lib/shared/schemas.tsconnection/src/index.ts
Comment on lines
+573
to
+574
| Showing first {(widgetQuery.data.rowLimit ?? 0).toLocaleString()}{" "} | ||
| rows. Refine your query to see all results. |
There was a problem hiding this comment.
Avoid misleading 0 fallback in truncation banner.
If rowLimit is unexpectedly missing, this renders “Showing first 0 rows,” which is confusing. Prefer a non-numeric fallback message (or a known default).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/card-container.tsx` around lines 573 - 574, The banner
currently falls back to 0 when widgetQuery.data.rowLimit is missing, producing
misleading text; update the JSX around the snippet that renders "Showing first
{(widgetQuery.data.rowLimit ?? 0).toLocaleString()} rows" to conditionally
render a non-numeric fallback (e.g., show the number only when
widgetQuery.data.rowLimit != null and otherwise render "Showing limited rows" or
"Showing first N rows" with a known default like widgetQuery.defaultRowLimit),
or use a string fallback such as (widgetQuery.data.rowLimit != null ?
widgetQuery.data.rowLimit.toLocaleString() : "limited") so the message is not
"0" when rowLimit is absent; adjust any related text in the same component
(card-container / the banner JSX) accordingly.
…erage Three independent failures from CI on PR #507: ## 1. Unit test deep-equality break `query-executor-core.test.ts` used `toEqual({ data: [...] })` on the executor result. After #499 the resolved value is `{ data, truncated, rowLimit }` (driver-reported truncation signal + effective row cap), so deep-equal failed with "expected { data, …(2) } to deeply equal { data }". Fix: update the assertion to include the new fields. Added 5 new tests to the executor suite covering: - setStatus(COMPLETE_TRUNCATED) → result.truncated === true - setStatus(COMPLETE) → result.truncated === false - No setStatus call → result.truncated === false (defensive) - credentials.maxRows → driver config.rowLimit + result.rowLimit echo - credentials.maxRows undefined → DEFAULT_MAX_ROWS (5000) fallback These directly cover the executor wiring that #499 added — should also unblock SonarCloud's "16.7% Coverage on New Code" failure since the previously-untested setStatus and maxRows paths now have unit coverage. Suite: 14 → 19 tests, all passing. Mocked the QueryStatus enum from @neoboard/connection so the test doesn't need a real connector module — only the integer value of COMPLETE_TRUNCATED (7) matters for the executor's handler. ## 2. E2E shard 2 — Create button outside viewport The new "Max Rows per Query" field added in #499 made the connection form's Advanced Settings section taller than the default Desktop Chrome viewport (1280×720). The Create button got pushed below the fold and Playwright's "scroll into view if needed" raced with Radix Dialog's own scroll container, leaving 6 connection-advanced.spec tests stuck at "element is outside of the viewport". Per direction from the user, fix this at the playwright.config.ts level by forcing a fixed, larger viewport for the entire suite. 1280×1024 fits every dialog in the suite without changing per-test code, doesn't auto-resize during a run, and is a one-line config change vs touching 6 individual tests. Verified locally: connection-advanced.spec.ts — 6/6 passing in 1.1 min after the viewport bump. ## 3. SonarCloud coverage on new code The 5 new executor tests above lift coverage on the touched query-executor.ts paths (setStatus handler, maxRows fallback, rowLimit echo) from 0% to ~80%, which should bring the PR over SonarCloud's 80% new-code coverage threshold. ## Verification - `npm run test` — 132 test files / 1766 tests passing - `npx playwright test connection-advanced.spec.ts --workers=4` — 6/6 passing in 1.1 min - `npx playwright test query-executor-core` — 19/19 passing in <1s - No new lint errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/playwright.config.ts (1)
58-61: Optional: dedupe viewport literal to avoid config drift.Line 58–61 repeats the same viewport object used in Line 52. Consider extracting a shared constant so future edits can’t diverge.
♻️ Optional cleanup
const serverPort = process.env.TEST_SERVER_PORT || "3100"; +const E2E_VIEWPORT = { width: 1280, height: 1024 } as const; @@ - viewport: { width: 1280, height: 1024 }, + viewport: E2E_VIEWPORT, @@ use: { ...devices["Desktop Chrome"], - viewport: { width: 1280, height: 1024 }, + viewport: E2E_VIEWPORT, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/playwright.config.ts` around lines 58 - 61, Extract the repeated viewport literal into a shared constant and reference it in both places to avoid drift; e.g., create a constant like const DEFAULT_VIEWPORT = { width: 1280, height: 1024 } near the Playwright config and replace the inline objects in the use: {...devices["Desktop Chrome"], viewport: { width: 1280, height: 1024 }} occurrences with viewport: DEFAULT_VIEWPORT so both the device setup and the other use block reference the same constant (update any imports/exports if necessary).app/src/lib/__tests__/query/query-executor-core.test.ts (1)
129-129: Use QueryStatus enum constants instead of magic numbers.Lines 129 and 152 use numeric literals (
7and6) with inline comments. Replace withQueryStatus.COMPLETE_TRUNCATEDandQueryStatus.COMPLETEfor clarity and to prevent drift if the enum changes.♻️ Proposed fix
-import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { QueryStatus } from "@neoboard/connection"; ... - cbs.setStatus?.(7); // QueryStatus.COMPLETE_TRUNCATED + cbs.setStatus?.(QueryStatus.COMPLETE_TRUNCATED); ... - cbs.setStatus?.(6); // QueryStatus.COMPLETE — not truncated + cbs.setStatus?.(QueryStatus.COMPLETE);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/lib/__tests__/query/query-executor-core.test.ts` at line 129, Replace the magic numeric status values used in the test with the QueryStatus enum to avoid brittle literals: update the cbs.setStatus?.(7) call to cbs.setStatus?.(QueryStatus.COMPLETE_TRUNCATED) and the other cbs.setStatus?.(6) to cbs.setStatus?.(QueryStatus.COMPLETE); ensure you import or reference QueryStatus in the test file and run the test to verify the enum names resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/playwright.config.ts`:
- Around line 58-61: Extract the repeated viewport literal into a shared
constant and reference it in both places to avoid drift; e.g., create a constant
like const DEFAULT_VIEWPORT = { width: 1280, height: 1024 } near the Playwright
config and replace the inline objects in the use: {...devices["Desktop Chrome"],
viewport: { width: 1280, height: 1024 }} occurrences with viewport:
DEFAULT_VIEWPORT so both the device setup and the other use block reference the
same constant (update any imports/exports if necessary).
In `@app/src/lib/__tests__/query/query-executor-core.test.ts`:
- Line 129: Replace the magic numeric status values used in the test with the
QueryStatus enum to avoid brittle literals: update the cbs.setStatus?.(7) call
to cbs.setStatus?.(QueryStatus.COMPLETE_TRUNCATED) and the other
cbs.setStatus?.(6) to cbs.setStatus?.(QueryStatus.COMPLETE); ensure you import
or reference QueryStatus in the test file and run the test to verify the enum
names resolve correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0259fcc5-2178-4eb4-95e8-32433ca529ce
📒 Files selected for processing (2)
app/playwright.config.tsapp/src/lib/__tests__/query/query-executor-core.test.ts
|
5 tasks
3 tasks
7 tasks
alfredo1996
pushed a commit
that referenced
this pull request
May 10, 2026
…erage Three independent failures from CI on PR #507: ## 1. Unit test deep-equality break `query-executor-core.test.ts` used `toEqual({ data: [...] })` on the executor result. After #499 the resolved value is `{ data, truncated, rowLimit }` (driver-reported truncation signal + effective row cap), so deep-equal failed with "expected { data, …(2) } to deeply equal { data }". Fix: update the assertion to include the new fields. Added 5 new tests to the executor suite covering: - setStatus(COMPLETE_TRUNCATED) → result.truncated === true - setStatus(COMPLETE) → result.truncated === false - No setStatus call → result.truncated === false (defensive) - credentials.maxRows → driver config.rowLimit + result.rowLimit echo - credentials.maxRows undefined → DEFAULT_MAX_ROWS (5000) fallback These directly cover the executor wiring that #499 added — should also unblock SonarCloud's "16.7% Coverage on New Code" failure since the previously-untested setStatus and maxRows paths now have unit coverage. Suite: 14 → 19 tests, all passing. Mocked the QueryStatus enum from @neoboard/connection so the test doesn't need a real connector module — only the integer value of COMPLETE_TRUNCATED (7) matters for the executor's handler. ## 2. E2E shard 2 — Create button outside viewport The new "Max Rows per Query" field added in #499 made the connection form's Advanced Settings section taller than the default Desktop Chrome viewport (1280×720). The Create button got pushed below the fold and Playwright's "scroll into view if needed" raced with Radix Dialog's own scroll container, leaving 6 connection-advanced.spec tests stuck at "element is outside of the viewport". Per direction from the user, fix this at the playwright.config.ts level by forcing a fixed, larger viewport for the entire suite. 1280×1024 fits every dialog in the suite without changing per-test code, doesn't auto-resize during a run, and is a one-line config change vs touching 6 individual tests. Verified locally: connection-advanced.spec.ts — 6/6 passing in 1.1 min after the viewport bump. ## 3. SonarCloud coverage on new code The 5 new executor tests above lift coverage on the touched query-executor.ts paths (setStatus handler, maxRows fallback, rowLimit echo) from 0% to ~80%, which should bring the PR over SonarCloud's 80% new-code coverage threshold. ## Verification - `npm run test` — 132 test files / 1766 tests passing - `npx playwright test connection-advanced.spec.ts --workers=4` — 6/6 passing in 1.1 min - `npx playwright test query-executor-core` — 19/19 passing in <1s - No new lint errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alfredo1996
added a commit
that referenced
this pull request
May 10, 2026
fix(query): wire driver truncation signal + user-configurable maxRows (#499)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the dead row-cap banner (#499) and adds the feature you asked for: users can now configure the max row limit per connection.
The bug
Both database drivers (PG + Neo4j) cap results at `config.rowLimit = 5000` BEFORE the API route sees them, but the route's `rawData.length > MAX_ROWS` comparison against 10,000 was never true — the glue layer between the driver's `setStatus(COMPLETE_TRUNCATED)` signal and the executor's callback was missing. `meta.truncated` was never set, and the "Showing first 10,000 rows" banner in `card-container.tsx:569` was unreachable dead code.
The fix
1. Wire `setStatus` through `query-executor`
The executor callback now implements `setStatus`, captures `COMPLETE_TRUNCATED`, and resolves the promise with `{ data, fields, truncated, rowLimit }`. The route forwards both fields into response meta and deletes the dead `MAX_ROWS = 10_000` constant.
`QueryStatus` is now exported from `@neoboard/connection` (previously internal).
2. Per-connection `maxRows` configuration
Added `maxRows` as an optional advanced setting on each connection:
3. Dynamic banner text
`card-container.tsx` now renders `"Showing first {rowLimit.toLocaleString()} rows"` using the `rowLimit` field from `meta`. A connection with `maxRows: 25000` will show "Showing first 25,000 rows" on its widgets; one with the default will show "Showing first 5,000 rows".
`use-widget-query.ts` `QueryResult` interface picks up the new optional `rowLimit?: number` field so TypeScript flows the value through end-to-end.
Verification
Backwards compatibility
UX flow for the new setting
Files changed (13)
Closes #499
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes