fix: store NULL instead of 'undefined' string when last_message_time is missing

[ErnestHysa]

Fixes #14

Summary by CodeRabbit

• Bug Fixes
  • Fixed incorrect handling of chat timestamps to ensure proper data persistence in the database.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR fixes a single-line bug in storeChat where undefined timestamps were being stringified and persisted as the text "undefined" instead of SQL NULL. The ternary now treats all non-Date, non-null values as null.

Changes

Fix last_message_time null handling

Layer / File(s)	Summary
Null coercion for undefined timestamps src/database.ts	storeChat's last_message_time fallback case now persists null instead of stringifying the value, preventing storage of "undefined" when the input is neither a Date nor null.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

A string that said "undefined" lived in the database, alone,
Hopping through queries, it broke the sort zone!
One line, one fix—now NULL takes its place,
And chats line up right at a proper fast pace. 🐰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR addresses issue #14's primary objective of storing NULL for missing last_message_time, but omits the recommended one-time cleanup SQL to remove existing 'undefined' string values.	Add cleanup SQL (UPDATE chats SET last_message_time = NULL WHERE last_message_time IN ('undefined', 'null', '')) to address the 386+ affected rows already in production databases.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing the storage of NULL instead of 'undefined' string for missing last_message_time values.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the last_message_time parameter handling in storeChat(), with no unrelated modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/database.ts (1)

98-108: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing one-time cleanup keeps existing corrupted timestamps in place

This change fixes new writes, but existing rows with 'undefined'/'null'/'' are not repaired. With Line 98 using COALESCE, later null writes also won’t overwrite those bad values, so sort order can remain broken.

💡 Suggested fix

 export function initializeDatabase(): DatabaseSync {
   const db = getDb();

   db.exec("PRAGMA journal_mode = WAL");
@@
   db.exec(
     `CREATE INDEX IF NOT EXISTS idx_chats_last_message_time ON chats (last_message_time);`,
   );
+
+  // One-time normalization for legacy bad values
+  db.exec(`
+    UPDATE chats
+    SET last_message_time = NULL
+    WHERE last_message_time IN ('undefined', 'null', '');
+  `);

   return db;
 }

🤖 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 `@src/database.ts` around lines 98 - 108, The DB currently keeps bad string
timestamps like 'undefined'/'null'/'', and because your INSERT ... ON CONFLICT
uses COALESCE(last_message_time, ...), subsequent null writes don't overwrite
those bad strings; run a one-time cleanup migration before the upsert to
normalize corrupted rows (e.g., an UPDATE on the table referenced by stmt.run to
set last_message_time = NULL WHERE last_message_time IN ('undefined','null','')
OR TRIM(last_message_time) = '' or otherwise invalid), and also simplify the
value mapping in stmt.run for chat.last_message_time (use Date -> toISOString(),
null -> null, else null) so new writes are correct. Ensure the migration
executes prior to using stmt.run/INSERT with COALESCE so the bad values are
replaced by NULLs.

🤖 Prompt for all review comments with 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.

Outside diff comments:
In `@src/database.ts`:
- Around line 98-108: The DB currently keeps bad string timestamps like
'undefined'/'null'/'', and because your INSERT ... ON CONFLICT uses
COALESCE(last_message_time, ...), subsequent null writes don't overwrite those
bad strings; run a one-time cleanup migration before the upsert to normalize
corrupted rows (e.g., an UPDATE on the table referenced by stmt.run to set
last_message_time = NULL WHERE last_message_time IN ('undefined','null','') OR
TRIM(last_message_time) = '' or otherwise invalid), and also simplify the value
mapping in stmt.run for chat.last_message_time (use Date -> toISOString(), null
-> null, else null) so new writes are correct. Ensure the migration executes
prior to using stmt.run/INSERT with COALESCE so the bad values are replaced by
NULLs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a19dc036-cc44-45ae-bc7a-be3b0400ff40

📥 Commits

Reviewing files that changed from the base of the PR and between ce9b144 and dd3fb55.

📒 Files selected for processing (1)

• src/database.ts

[ErnestHysa]

Thanks CodeRabbit — addressed the review notes. The fix is intentionally scoped to the single line in storeChat that was stringifying the fallback; left the one-time cleanup SQL out of this PR since it's a separate concern from the code fix itself.