[FEATURE] Add unit tests for core functionality #934

cubic-dev-ai · 2026-07-23T16:03:24Z

P2: Silently catching all exceptions from unlinkSync can hide real problems. If the file exists but removal fails for a reason other than ENOENT (e.g., permission denied, file locked), the test will proceed with a stale database and may produce misleading results. Consider re-throwing non-ENOENT errors:

try { fs.unlinkSync(dbPath); } catch (e: any) { if (e.code !== 'ENOENT') throw e; }

The same applies to the afterAll cleanup block.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/dailyRefinement.test.ts, line 18: <comment>Silently catching all exceptions from `unlinkSync` can hide real problems. If the file exists but removal fails for a reason other than `ENOENT` (e.g., permission denied, file locked), the test will proceed with a stale database and may produce misleading results. Consider re-throwing non-`ENOENT` errors: ```ts try { fs.unlinkSync(dbPath); } catch (e: any) { if (e.code !== 'ENOENT') throw e; }

The same applies to the afterAll cleanup block.
@@ -14,6 +14,9 @@ describe('Daily Civic Intelligence Refinement Engine', () => { beforeAll((done) => { dbPath = path.resolve(__dirname, 'test_issues.db'); + if (fs.existsSync(dbPath)) { + try { fs.unlinkSync(dbPath); } catch (e) {} + } const db = new sqlite3.Database(dbPath); ```

coderabbitai · 2026-07-23T15:55:06Z

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "== files ==" git ls-files | rg '(^|/)dailyRefinement\.test\.ts$|dailyRefinement|issues\.db|test_issues\.db' || true echo "== target file outline/size ==" if [ -f tests/dailyRefinement.test.ts ]; then wc -l tests/dailyRefinement.test.ts ast-grep outline tests/dailyRefinement.test.ts --view compact || true echo "== target file relevant sections ==" sed -n '1,90p' tests/dailyRefinement.test.ts | cat -n fi echo "== SQLite/database references in tests ==" rg -n "test_issues\.db|createClient|close|unlinkSync|existsSync|dbPath|sqlite|SQLite|dailyRefinement" tests src . 2>/dev/null | head -200 || true

Repository: RohanExploit/VishwaGuru

Length of output: 16083

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "== trendAnalyzer outline ==" ast-grep outline services/trendAnalyzer.ts --view signatures || true echo "== trendAnalyzer relevant sections ==" sed -n '1,110p' services/trendAnalyzer.ts | cat -n echo "== package test scripts ==" cat package.json | jq '{scripts: .scripts, dependencies: {sqlite3:.dependencies.sqlite3}, devDependencies:.devDependencies}'

Repository: RohanExploit/VishwaGuru

Length of output: 3607

Do not silently ignore SQLite cleanup failures.

Both lifecycle boundaries can leave test_issues.db behind while the test suite still reports success:

beforeAll: fail setup on any unlinkSync error except ENOENT instead of proceeding with a corrupted/stale database.

afterAll: close the TrendAnalyzer callback-backed database before unlinking and propagate non-ENOENT cleanup errors instead of relying on a 500ms timer.

📍 Affects 1 file

tests/dailyRefinement.test.ts#L17-L19 (this comment)

tests/dailyRefinement.test.ts#L37-L43

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/dailyRefinement.test.ts` around lines 17 - 19, Update both database cleanup sites in tests/dailyRefinement.test.ts: in beforeAll, let unlinkSync failures propagate unless the error code is ENOENT; in afterAll, explicitly close the TrendAnalyzer callback-backed database before unlinking, remove the 500ms timer workaround, and propagate any non-ENOENT unlinkSync failure.

coderabbitai · 2026-07-23T15:55:06Z

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "== locate file ==" fd -a dailyRefinement.test.ts . || true echo "== inspect file around relevant lines ==" if [ -f tests/dailyRefinement.test.ts ]; then cat -n tests/dailyRefinement.test.ts | sed -n '1,120p' fi echo "== package hints for sqlite dependency ==" for f in package.json pnpm-lock.yaml package-lock.json yarn.lock; do [ -f "$f" ] && { echo "--- $f"; rg -n "sqlite3|better-sqlite3|node-sqlite3" "$f" || true; } done

Repository: RohanExploit/VishwaGuru

Length of output: 5816

🏁 Script executed:

#!/bin/bash set -euo pipefail # Inspect whether node-sqlite3's API signature for CREATE TABLE/run/close/use callbacks is present in the repository # as installed docs or examples, and compare common callback behavior from local package metadata/docs if present. python3 - <<'PY' from pathlib import Path paths=[] for root in [Path('.')]: for p in root.rglob('*sqlite3*'): if p.is_file(): paths.append(str(p)) print("sqlite3-related files:", "\n".join(paths[:20]) if paths else "(none)") for p in [Path('node_modules/sqlite3/package.json'), Path('node_modules/sqlite3/LICENSE.md'), Path('node_modules/better-sqlite3/package.json')]: if p.exists(): print(f"--- {p} first 20 lines") lines=p.read_text(encoding='utf-8', errors='replace').splitlines()[:20] print("\n".join(lines)) PY echo "== any local documentation examples ==" rg -n "stmt\\.run|CREATE TABLE|db\\.close|db\\.serialize|node-sqlite3|better-sqlite3" . \ -g '!node_modules' -g '!dist' -g '!build' -g '!coverage' \ -C 3 || true

Repository: RohanExploit/VishwaGuru

Length of output: 11639

🌐 Web query:

node-sqlite3 v6 Database.prototype run Statement.prototype run finalize close callback error signature

💡 Result:

In the node-sqlite3 library, the callback error signature for Database#run, Statement#run, Database#close, and Statement#finalize generally follows the standard Node.js convention where the first argument is an error object (or null if no error occurred) [1][2][3]. Specifically, the callback signatures are as follows: Database.prototype.run and Statement.prototype.run The callback signature is function(err). If an error occurs, the first parameter is an Error object; if execution is successful, the first parameter is null [1][2]. Note that unlike some other Node.js libraries, result data for run is not passed as an argument to the callback; instead, relevant information (such as changes or lastID) is accessed via the 'this' context of the callback function [1][4]. Because of this binding, you should avoid using arrow functions for these callbacks if you need to access that context, as arrow functions lexically bind 'this' [5][4]. Database.prototype.close The callback signature is function(err). The err parameter will be an Error object if the database fails to close, or null if it closes successfully [2][6]. Statement.prototype.finalize The callback signature is function(err). Similar to the others, the first parameter is an Error object if an error occurs during finalization, or null otherwise [2]. Summary of callback error behavior: - If a callback is provided, errors are passed as the first argument [1][3]. - If no callback is provided, errors are typically emitted as 'error' events on the database or statement object [1][7][3]. - For run specifically, remember that data is returned via the 'this' object, not as a second argument to the callback [1][4].

Citations:

1: https://github.com/TryGhost/node-sqlite3/wiki/API

2: https://github.com/TryGhost/node-sqlite3/blob/master/lib/sqlite3.d.ts

3: https://stackoverflow.com/questions/32980459/error-handling-in-node-sqlite3

4: odyssee of a noob trying to get the ID of inserted row TryGhost/node-sqlite3#622

5: https://stackoverflow.com/questions/10022984/nodejs-node-sqlite3-run-callback-not-working

6: db.close() never finishes on a DB with invalid path TryGhost/node-sqlite3#1617

7: https://github.com/mapbox/node-sqlite3/blob/master/src/statement.cc

Propagate fixture-seeding errors into Jest.

db.run and stmt.run callback on errors, but these calls omit callbacks so table creation or inserts can fail without calling beforeAll’s done; only db.close(done) reports the close operation. Add callbacks for these setup operations that pass the first error to done, and only close when the seeding pipeline succeeds.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/dailyRefinement.test.ts` around lines 31 - 33, Update the beforeAll fixture-seeding flow in tests/dailyRefinement.test.ts, including the db.run and stmt.run operations around stmt.finalize, to propagate each operation’s first error to Jest’s done callback and stop the pipeline on failure. Only invoke db.close(done) after table creation and inserts complete successfully, preserving the existing successful cleanup behavior.

Source: MCP tools

cubic-dev-ai · 2026-07-23T16:03:24Z

P2: Database setup can report success even when statement finalization fails, leaving incomplete seed data and making the test pass or fail for the wrong reason. Preserving the finalize error before closing the database keeps setup failures visible.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/dailyRefinement.test.ts, line 31: <comment>Database setup can report success even when statement finalization fails, leaving incomplete seed data and making the test pass or fail for the wrong reason. Preserving the finalize error before closing the database keeps setup failures visible.</comment> <file context> @@ -25,12 +28,19 @@ describe('Daily Civic Intelligence Refinement Engine', () => { stmt.run('Another pothole here', 'infrastructure', 'Ward 1', now); - stmt.finalize(done); + stmt.finalize(() => { + db.close(done); + }); </file context>

Suggested change

stmt.finalize(() => {

db.close(done);

});

stmt.finalize((err) => {

if (err) return done(err);

db.close(done);

});

cubic-dev-ai · 2026-07-23T16:03:24Z

P2: Teardown is still racy: a fixed delay does not prove that SQLite released the lock, and cleanup failure can be hidden after the delay. Awaiting the analyzer/database close completion and reporting or retrying unlink errors would prevent leaked test artifacts and cross-run contamination.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/dailyRefinement.test.ts, line 43: <comment>Teardown is still racy: a fixed delay does not prove that SQLite released the lock, and cleanup failure can be hidden after the delay. Awaiting the analyzer/database close completion and reporting or retrying unlink errors would prevent leaked test artifacts and cross-run contamination.</comment> <file context> @@ -25,12 +28,19 @@ describe('Daily Civic Intelligence Refinement Engine', () => { + try { fs.unlinkSync(dbPath); } catch (e) {} + } + done(); + }, 500); // wait for locks to release }); </file context>

Navigation Menu

[FEATURE] Add unit tests for core functionality #934

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

cubic-dev-ai Bot Jul 23, 2026

Uh oh!

coderabbitai Bot Jul 23, 2026

Uh oh!

coderabbitai Bot Jul 23, 2026

Uh oh!

cubic-dev-ai Bot Jul 23, 2026

Uh oh!

cubic-dev-ai Bot Jul 23, 2026

Uh oh!

Uh oh!

[FEATURE] Add unit tests for core functionality #934

Uh oh!

[FEATURE] Add unit tests for core functionality #934

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

cubic-dev-ai Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!