refactor(ui): extract testable batch-update logic from sidebar.js by zknpr · Pull Request #416 · zknpr/SQLite-Explorer

zknpr · 2026-05-30T20:25:12Z

What

Reworks the closed #360/#361 so they're actually verifiable. Those PRs extracted batch-update helpers but kept them inside sidebar.js, which can't be unit-tested (its import chain needs a DOM). This instead pulls the pure value-processing logic into a DOM-free module with unit tests.

core/ui/modules/batch-update-logic.js (+ .d.ts) — groupSelectedCellsByColumn, summarizeColumnValue, prepareBatchUpdates (type coercion / NULL / json_patch / skip-empty)
sidebar.js — updateBatchSidebar + applyBatchUpdate delegate to these (behavior unchanged); dropped a now-unused escapeHtml import
tests/unit/batch-update-logic.test.ts — 13 cases

Verification

npm test → 324 passing (was 311); tsc --noEmit → 0 errors
Runtime (web demo): selected 2 Title cells → batch panel showed the grouped Title field with placeholder (mixed values) → entered a value → both cells updated; zero console errors

🤖 Generated with Claude Code

Reworks the closed #360/#361 (which couldn't be unit-tested as in-file helpers — sidebar.js's import chain requires a DOM). Extracts the pure value-processing logic into a DOM-free module so it can be unit-tested: - core/ui/modules/batch-update-logic.js (+ .d.ts): groupSelectedCellsByColumn, summarizeColumnValue, prepareBatchUpdates (type coercion / NULL / json_patch / skip-empty rules) - sidebar.js: updateBatchSidebar + applyBatchUpdate delegate to these (behavior unchanged); removed a now-unused escapeHtml import - tests/unit/batch-update-logic.test.ts: 13 cases covering the rules Verified: 324 unit tests pass (was 311); tsc --noEmit clean; full batch-update flow runtime-checked in the web demo (select 2 cells -> panel shows the grouped column with "(mixed values)" -> apply -> both cells update; zero console errors). Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

vercel · 2026-05-30T20:25:14Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
sq-lite-explorer	Ready	Preview, Comment	May 30, 2026 9:00pm

coderabbitai · 2026-05-30T20:25:18Z

Warning

Review limit reached

@zknpr, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 37 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 93f234fa-7c0e-4b12-914c-74f974ccf2e8

📥 Commits

Reviewing files that changed from the base of the PR and between 04e8e34 and 9ea707a.

📒 Files selected for processing (6)

core/ui/modules/batch-update-logic.d.ts
core/ui/modules/batch-update-logic.js
core/ui/modules/sidebar.js
core/ui/viewer.html
tests/unit/batch-update-logic.test.ts
website/public/sqlite-viewer/viewer.html

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch chore/batch-update-refactor

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

gemini-code-assist

Code Review

This pull request extracts DOM-free batch-update logic from sidebar.js into a new module batch-update-logic.js along with its TypeScript declarations, and introduces comprehensive unit tests in batch-update-logic.test.ts. The review feedback focuses on improving the robustness of the extracted helper functions. Specifically, it suggests adding defensive checks to prevent potential TypeErrors when tableColumns or input.dataset are undefined, handling empty value sets in summarizeColumnValue to avoid returning 'undefined', and normalizing column types to uppercase to ensure case-insensitive type coercion.

gemini-code-assist · 2026-05-30T20:26:16Z

+    for (const cell of selectedCells) {
+        if (!columns.has(cell.colIdx)) {
+            const colDef = tableColumns[cell.colIdx];
+            columns.set(cell.colIdx, {
+                name: colDef.name,
+                type: colDef.type,
+                values: new Set()
+            });
+        }


If tableColumns is undefined or cell.colIdx is out of bounds, colDef will be undefined, leading to a TypeError when accessing colDef.name. Adding a defensive check prevents potential runtime crashes.

Suggested change

for (const cell of selectedCells) {

if (!columns.has(cell.colIdx)) {

const colDef = tableColumns[cell.colIdx];

columns.set(cell.colIdx, {

name: colDef.name,

type: colDef.type,

values: new Set()

});

}

for (const cell of selectedCells) {

const colDef = tableColumns && tableColumns[cell.colIdx];

if (!colDef) continue;

if (!columns.has(cell.colIdx)) {

columns.set(cell.colIdx, {

name: colDef.name,

type: colDef.type,

values: new Set()

});

}

gemini-code-assist · 2026-05-30T20:26:16Z

+export function summarizeColumnValue(values) {
+    const uniqueValues = Array.from(values);
+    if (uniqueValues.length > 1) return '(mixed values)';
+    const val = uniqueValues[0];
+    if (val === null) return 'NULL';
+    if (val instanceof Uint8Array) return '[BLOB]';
+    return String(val);
+}


If values is empty, uniqueValues.length is 0, and uniqueValues[0] is undefined. This causes the function to return the string 'undefined'. Handling the empty case explicitly avoids returning an unexpected string.

Suggested change

export function summarizeColumnValue(values) {

const uniqueValues = Array.from(values);

if (uniqueValues.length > 1) return '(mixed values)';

const val = uniqueValues[0];

if (val === null) return 'NULL';

if (val instanceof Uint8Array) return '[BLOB]';

return String(val);

}

export function summarizeColumnValue(values) {

const uniqueValues = Array.from(values || []);

if (uniqueValues.length === 0) return '';

if (uniqueValues.length > 1) return '(mixed values)';

const val = uniqueValues[0];

if (val === null) return 'NULL';

if (val instanceof Uint8Array) return '[BLOB]';

return String(val);

}

gemini-code-assist · 2026-05-30T20:26:17Z

+        const isNull = input.dataset.isnull === 'true';
+        const isPatch = input.dataset.ispatch === 'true';


If input is a mock or does not have a dataset property defined, accessing input.dataset.isnull will throw a TypeError. Safeguarding this with a fallback object improves reliability, especially in unit tests.

Suggested change

const isNull = input.dataset.isnull === 'true';

const isPatch = input.dataset.ispatch === 'true';

const dataset = input.dataset || {};

const isNull = dataset.isnull === 'true';

const isPatch = dataset.ispatch === 'true';

gemini-code-assist · 2026-05-30T20:26:17Z

+        // Skip cells left blank unless they were explicitly set to NULL.
+        if (value === '' && !isNull) continue;
+
+        const colDef = tableColumns[cell.colIdx];


If tableColumns is undefined or cell.colIdx is out of bounds, colDef will be undefined, leading to a TypeError when accessing colDef.type or colDef.name. Adding a defensive check ensures robustness.

Suggested change

const colDef = tableColumns[cell.colIdx];

const colDef = tableColumns && tableColumns[cell.colIdx];

if (!colDef) continue;

gemini-code-assist · 2026-05-30T20:26:17Z

+        } else if (colDef.type === 'INTEGER' || colDef.type === 'REAL' || colDef.type === 'NUMERIC') {
+            // Coerce numeric column types when the input parses as a number.
+            if (!isNaN(Number(value)) && value.trim() !== '') {
+                finalValue = Number(value);
+            }
+        }


The column type check is case-sensitive (INTEGER, REAL, NUMERIC). Depending on the database driver or how the schema is defined, column types might be returned in lowercase (e.g., integer). Normalizing the type to uppercase ensures robust type coercion.

} else { const type = colDef.type ? colDef.type.toUpperCase() : ''; if (type === 'INTEGER' || type === 'REAL' || type === 'NUMERIC') { // Coerce numeric column types when the input parses as a number. if (!isNaN(Number(value)) && value.trim() !== '') { finalValue = Number(value); } } }

Applies the defensive findings from Gemini's review of this PR: - groupSelectedCellsByColumn / prepareBatchUpdates: skip cells whose colDef is missing (stale/out-of-bounds selection, e.g. after a column drop) rather than throwing - summarizeColumnValue: return '' for an empty value set - prepareBatchUpdates: tolerate an input without a `dataset` - `dataset` made optional in the .d.ts + 4 edge-case unit tests (suite: 328 passing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- batch-update-logic.js: normalize column type to upper-case before numeric coercion (Gemini — SQLite reports declared types verbatim, e.g. 'integer', so the case-sensitive check could miss lowercase numeric columns) - sidebar.js: guard colDef?.name in the invalid-JSON validation path against a stale/out-of-bounds colIdx (Codex non-blocking caveat) - tests: add NUMERIC + lowercase-type coercion cases (suite: 330 passing) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

zknpr · 2026-05-30T21:04:43Z

👍 Codex review: APPROVE — no blocking findings (behavioral equivalence preserved; missing-colDef guards + uppercase type-normalization correct; test coverage adequate; tsc --noEmit exit 0).

All 5 Gemini findings addressed (colDef guards in group/prepare, empty-set handling, dataset fallback, case-insensitive numeric coercion).
CI green: build + typecheck + 330 unit tests + Vercel.
Note: Codex's first-pass npm test EPERM was a sandbox limitation (tsx IPC pipe), refuted by green CI + local 330/0; CodeRabbit couldn't review (org out of credits).

Merging.

vercel Bot deployed to Preview May 30, 2026 20:25 View deployment

gemini-code-assist Bot reviewed May 30, 2026

View reviewed changes

vercel Bot deployed to Preview May 30, 2026 20:45 View deployment

vercel Bot deployed to Preview May 30, 2026 21:00 View deployment

zknpr merged commit cb011bf into main May 30, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(ui): extract testable batch-update logic from sidebar.js#416

refactor(ui): extract testable batch-update logic from sidebar.js#416
zknpr merged 3 commits into
mainfrom
chore/batch-update-refactor

zknpr commented May 30, 2026

Uh oh!

vercel Bot commented May 30, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 30, 2026 •

edited

Loading

Review limit reached

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

gemini-code-assist Bot May 30, 2026

Uh oh!

zknpr commented May 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		const isNull = input.dataset.isnull === 'true';
		const isPatch = input.dataset.ispatch === 'true';

-        const isNull = input.dataset.isnull === 'true';
-        const isPatch = input.dataset.ispatch === 'true';
+        const dataset = input.dataset || {};
+        const isNull = dataset.isnull === 'true';
+        const isPatch = dataset.ispatch === 'true';

	const colDef = tableColumns[cell.colIdx];
	const colDef = tableColumns && tableColumns[cell.colIdx];
	if (!colDef) continue;

Uh oh!

Conversation

zknpr commented May 30, 2026

What

Verification

Uh oh!

vercel Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

zknpr commented May 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented May 30, 2026 •

edited

Loading

coderabbitai Bot commented May 30, 2026 •

edited

Loading