Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/internal/collection.ts`:
- Around line 10-12: The dynamic delete usage (delete item[key]) in the
normalization block that checks key !== 'meta' and parsed being an empty object
should be replaced by assigning item[key] = undefined (or 'NULL') and then
filtering those undefined/'NULL' entries out when returning the normalized
object; update the logic in the same function handling item, key, parsed (and
the similar occurrence around lines 18-20) so you mark fields to omit by setting
them to undefined rather than deleting, then at the end produce a new object
that excludes keys whose value is undefined or the sentinel 'NULL' to preserve
the “field omitted” behavior without using dynamic delete.
- Around line 17-23: The boolean coercion branch in the collection normalization
treats the sentinel string 'NULL' as truthy (using Boolean(item[key])), causing
optional boolean fields to be kept instead of omitted; update the logic in the
boolean handling for item[key] (the branch that checks fields[key] ===
'boolean') to first treat the exact string 'NULL' as empty (delete the key or
treat as null) before any Boolean(...) conversion, and apply the same sentinel
check to the later normalization block that handles null/undefined/'NULL' to
ensure 'NULL' never gets coerced to true; look for uses of fields, item and key
in the boolean branch and the subsequent normalization block and add the
sentinel check there.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ccba6021-110e-401e-9fe1-e5d6b023a964

Reviewing files that changed from the base of the PR and between 55b8793 and 72a12b0.

• src/runtime/internal/collection.ts

src/runtime/internal/collection.ts (1)

17-24: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

'NULL' sentinel is coerced to true in the boolean branch — still unresolved from prior review

'NULL' (the SQL NULL sentinel string) is truthy, so item[key] == null is false, and Boolean('NULL') evaluates to true. The sentinel normalization loop (Lines 27–31) only runs after this branch, so optional boolean fields backed by a SQL NULL can be injected as true instead of being removed.

Extend the null-guard at Line 18 to also cover the 'NULL' sentinel:

🔧 Proposed fix

-      if (item[key] == null) {
+      if (item[key] == null || item[key] === 'NULL') {
         Reflect.deleteProperty(item, key)
       }

🤖 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/runtime/internal/collection.ts` around lines 17 - 24, In the boolean
normalization branch (where fields[key as string] === 'boolean') extend the
null-guard so SQL NULL sentinels are treated like null: instead of only checking
item[key] == null, also check for the string 'NULL' and delete the property in
that case; otherwise continue to coerce with Boolean(item[key]). Update the
check around item[key] in that branch (use item[key] == null || item[key] ===
'NULL') so optional boolean fields backed by the SQL 'NULL' sentinel are removed
rather than coerced to true.

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/runtime/internal/collection.ts`:
- Line 4: The TypeScript error is caused by using Reflect.deleteProperty with an
unconstrained generic T; update the function signature of refineContentFields to
constrain T to object (i.e., change to refineContentFields<T extends
object>(...)) so that doc/item are accepted by Reflect.deleteProperty; modify
only the generic constraint on the refineContentFields function to resolve the
TS2345 failures while leaving the function body unchanged.

---

Duplicate comments:
In `@src/runtime/internal/collection.ts`:
- Around line 17-24: In the boolean normalization branch (where fields[key as
string] === 'boolean') extend the null-guard so SQL NULL sentinels are treated
like null: instead of only checking item[key] == null, also check for the string
'NULL' and delete the property in that case; otherwise continue to coerce with
Boolean(item[key]). Update the check around item[key] in that branch (use
item[key] == null || item[key] === 'NULL') so optional boolean fields backed by
the SQL 'NULL' sentinel are removed rather than coerced to true.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1547fc42-3829-46ec-8168-f751e73edd36

Reviewing files that changed from the base of the PR and between 72a12b0 and b279c22.

• src/runtime/internal/collection.ts

Actionable comments posted: 1

src/runtime/internal/collection.ts (1)

17-24: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

'NULL' sentinel still coerced to true in the boolean branch (unresolved from prior review)

The new null guard at line 18 correctly handles JS null, but the string 'NULL' (returned by D1/SQLite for absent columns) is neither == null nor === 'undefined', so it falls through to Boolean('NULL') === true. By the time the 'NULL'-normalization loop (lines 27–31) runs, the key already holds true and is never cleaned up.

🐛 Proposed fix

-      if (item[key] == null) {
+      if (item[key] == null || item[key] === 'NULL') {
         Reflect.deleteProperty(item as object, key)
       }

🤖 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/runtime/internal/collection.ts` around lines 17 - 24, The boolean branch
in collection.ts (the block using fields, item, key and Reflect.deleteProperty)
currently coerces the string sentinel 'NULL' to true; change the guard so that
values equal to null, the string 'NULL', or the string 'undefined' are treated
as absent and removed (use Reflect.deleteProperty(item as object, key)),
otherwise coerce to Boolean as before (item[key] = Boolean(item[key]) as never).
Ensure you update only that branch so the subsequent 'NULL'-normalization loop
no longer has to handle already-coerced booleans.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/internal/collection.ts`:
- Around line 8-15: The loop that parses JSON fields (references: fields, item,
key and the JSON.parse call) only guards against the literal string 'undefined'
so values equal to 'NULL' get passed to JSON.parse and throw; update the guard
to also exclude the string 'NULL' (i.e., treat item[key] === 'NULL' like
'undefined') and additionally wrap the JSON.parse call in a try/catch so
malformed JSON doesn't propagate a SyntaxError—on parse failure either log the
error and leave the original value or set the field to undefined/omit it
similarly to how 'undefined' is handled.

---

Duplicate comments:
In `@src/runtime/internal/collection.ts`:
- Around line 17-24: The boolean branch in collection.ts (the block using
fields, item, key and Reflect.deleteProperty) currently coerces the string
sentinel 'NULL' to true; change the guard so that values equal to null, the
string 'NULL', or the string 'undefined' are treated as absent and removed (use
Reflect.deleteProperty(item as object, key)), otherwise coerce to Boolean as
before (item[key] = Boolean(item[key]) as never). Ensure you update only that
branch so the subsequent 'NULL'-normalization loop no longer has to handle
already-coerced booleans.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 94011a21-9022-4691-bbc3-58705fab08a1

Reviewing files that changed from the base of the PR and between b279c22 and 4e2a322.

• src/runtime/internal/collection.ts

src/runtime/internal/collection.ts (2)

22-29: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

'NULL' sentinel is coerced to true in the boolean branch.

At Line 22 the guard only filters the string 'undefined'. For item[key] === 'NULL', item[key] == null is false (Line 23), so execution falls through to Boolean('NULL') → true at Line 27. The optional boolean field then leaks as true instead of being omitted. The 'NULL' normalization later (lines 32–36) cannot recover because the value is already a real boolean by then.

🐛 Proposed fix

-    if (fields[key as string] === 'boolean' && item[key] !== 'undefined') {
-      if (item[key] == null) {
+    if (fields[key as string] === 'boolean' && item[key] !== 'undefined' && item[key] !== 'NULL') {
+      if (item[key] == null) {
         Reflect.deleteProperty(item as object, key)
       }
       else {
         item[key] = Boolean(item[key]) as never
       }
     }
+    else if (fields[key as string] === 'boolean' && item[key] === 'NULL') {
+      Reflect.deleteProperty(item as object, key)
+    }

Or, more simply, normalize 'NULL' (and 'undefined') sentinels into null/undefined before the type-specific branches, eliminating the duplicated guards entirely.

🤖 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/runtime/internal/collection.ts` around lines 22 - 29, The boolean branch
mistakenly coerces the string sentinel 'NULL' to true; update the logic in
collection handling (around the fields/item/key processing) to normalize
sentinel strings ('NULL' and 'undefined') to null/undefined before any
type-specific branches (or, alternatively, extend the boolean guard to treat
'NULL' as null), then delete the property via Reflect.deleteProperty when
sentinel indicates omission so optional booleans are omitted instead of becoming
true; adjust the code that currently uses Boolean(item[key]) and the later NULL
normalization so sentinels are handled only once up-front.

---

13-21: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

'NULL' sentinel still bypasses the JSON guard and crashes JSON.parse.

The guard at Line 13 only excludes 'undefined'. The string 'NULL' is truthy and passes through, so JSON.parse('NULL') at Line 14 throws SyntaxError (uppercase NULL is not valid JSON). The 'NULL' → undefined normalization at lines 32–36 runs after this loop, so it cannot rescue the field. Additionally, JSON.parse is unguarded — any malformed row throws an unhandled exception.

🐛 Proposed fix

-    if (fields[key as string] === 'json' && item[key] && item[key] !== 'undefined') {
-      const parsed = JSON.parse(item[key] as string)
-      if (key !== 'meta' && parsed && typeof parsed === 'object' && !Array.isArray(parsed) && Object.keys(parsed).length === 0) {
-        Reflect.deleteProperty(item as object, key)
-      }
-      else {
-        item[key] = parsed
-      }
-    }
+    if (fields[key as string] === 'json' && item[key] && item[key] !== 'undefined' && item[key] !== 'NULL') {
+      let parsed: unknown
+      try {
+        parsed = JSON.parse(item[key] as string)
+      }
+      catch {
+        // Leave the original value in place if parsing fails.
+        continue
+      }
+      if (key !== 'meta' && parsed && typeof parsed === 'object' && !Array.isArray(parsed) && Object.keys(parsed as object).length === 0) {
+        Reflect.deleteProperty(item as object, key)
+      }
+      else {
+        item[key] = parsed as never
+      }
+    }

🤖 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/runtime/internal/collection.ts` around lines 13 - 21, The loop currently
calls JSON.parse on any truthy item[key] (e.g., "NULL") and can throw; update
the block that handles fields[key] === 'json' to (1) explicitly treat sentinel
strings like 'NULL' (and optionally other non-JSON sentinels) as absent so they
don't reach JSON.parse, and (2) wrap JSON.parse(item[key] as string) in a
try/catch; on parse failure or sentinel hit, normalize the field to undefined
(or delete the property using the existing empty-object rule for non-'meta'
keys), otherwise assign the parsed value and keep the existing logic that
deletes empty objects for non-meta keys (use the same identifiers: fields, key,
item, parsed, meta).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@src/runtime/internal/collection.ts`:
- Around line 22-29: The boolean branch mistakenly coerces the string sentinel
'NULL' to true; update the logic in collection handling (around the
fields/item/key processing) to normalize sentinel strings ('NULL' and
'undefined') to null/undefined before any type-specific branches (or,
alternatively, extend the boolean guard to treat 'NULL' as null), then delete
the property via Reflect.deleteProperty when sentinel indicates omission so
optional booleans are omitted instead of becoming true; adjust the code that
currently uses Boolean(item[key]) and the later NULL normalization so sentinels
are handled only once up-front.
- Around line 13-21: The loop currently calls JSON.parse on any truthy item[key]
(e.g., "NULL") and can throw; update the block that handles fields[key] ===
'json' to (1) explicitly treat sentinel strings like 'NULL' (and optionally
other non-JSON sentinels) as absent so they don't reach JSON.parse, and (2) wrap
JSON.parse(item[key] as string) in a try/catch; on parse failure or sentinel
hit, normalize the field to undefined (or delete the property using the existing
empty-object rule for non-'meta' keys), otherwise assign the parsed value and
keep the existing logic that deletes empty objects for non-meta keys (use the
same identifiers: fields, key, item, parsed, meta).

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8214a9bf-89e1-452e-bbce-65beb408a0c7

Reviewing files that changed from the base of the PR and between 4e2a322 and c9abd1d.

• src/runtime/internal/collection.ts

Thanks for the PR @surohak,
Do you mind adding tests to verify the changes?