Skip to content

Conversation

@findolor
Copy link
Collaborator

@findolor findolor commented Sep 17, 2025

Motivation

See issue: #13

Solution

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

fix #13

Summary by CodeRabbit

  • New Features

    • Added FLOAT_IS_ZERO SQL function to detect zero values from hex-encoded floats.
  • Bug Fixes

    • FLOAT_NEGATE now returns NULL when given NULL input.
  • Refactor

    • Centralized and standardized error message constants across bigint, float sum/negate, and math functions (no behavior changes).
  • Tests

    • Added comprehensive tests validating FLOAT_IS_ZERO behavior, error cases, NULL handling, and integrations.
  • Chores

    • Updated example app to use the latest package build; minor package metadata formatting.

@findolor findolor self-assigned this Sep 17, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 17, 2025

Walkthrough

Adds a new SQLite scalar function FLOAT_IS_ZERO with Rust FFI and helper logic to parse hex-encoded floats and detect zero. Registers the function, adds JS/TS tests, centralizes several error-message literals into constants, and adjusts FLOAT_NEGATE to return NULL for NULL inputs. Minor package.json updates.

Changes

Cohort / File(s) Summary
New FLOAT_IS_ZERO function (core)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs, packages/sqlite-web-core/src/database_functions/mod.rs
Adds float_is_zero FFI function and float_is_zero_hex helper; registers FLOAT_IS_ZERO (1 arg). Validates arg count, handles NULL input, parses hex via Float::from_hex, returns 1/0 for zero/non-zero, and reports parse/arg errors.
JS tests for FLOAT_IS_ZERO
svelte-test/tests/database-functions/float-is-zero.test.ts
New Vitest suite covering function availability, zero detection, whitespace handling, NULL behavior, interactions with other functions, aggregation cases, and error cases for empty/invalid hex.
Error message constantization
packages/sqlite-web-core/src/database_functions/bigint_sum.rs, .../float_sum.rs, .../rain_math.rs, .../float_negate.rs
Replaces inline C-string literals with module-level NUL-terminated constants for error messages; float_negate also adds an early NULL-input check returning NULL.
Test cleanup update
svelte-test/tests/fixtures/test-helpers.ts
Includes float_is_zero_test table in cleanup routine to drop after tests.
Package metadata
pkg/package.json, svelte-test/package.json
Adds trailing newline to pkg/package.json; updates test app dependency tarball for @rainlanguage/sqlite-web to alpha.4.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client as SQL Query
  participant SQLite as SQLite Engine
  participant FFI as float_is_zero (Rust)
  participant Parser as Float::from_hex

  Client->>SQLite: SELECT FLOAT_IS_ZERO(value)
  SQLite->>FFI: xFunc(context, argc, argv)
  alt argc != 1
    FFI-->>SQLite: sqlite3_result_error("requires exactly 1 argument")
  else if arg is NULL
    FFI-->>SQLite: sqlite3_result_null()
  else
    FFI->>Parser: parse hex string
    alt parse error
      FFI-->>SQLite: sqlite3_result_error("failed to parse hex")
    else parsed
      FFI-->>SQLite: sqlite3_result_int(is_zero ? 1 : 0)
    end
  end
  SQLite-->>Client: result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Suggested labels

codex

Suggested reviewers

  • hardyjosh
  • 0xgleb

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The changeset includes edits unrelated to adding FLOAT_IS_ZERO: several modules (bigint_sum.rs, float_sum.rs, rain_math.rs) were refactored to use module-level error-message constants and package metadata/test fixtures were bumped, which are mostly cosmetic, but float_negate.rs was modified to early-return on NULL (changing behavior) and that behavioral change is not justified by issue #13, so the PR mixes the intended feature with unrelated refactors and at least one behavior-altering change. Separate the FLOAT_IS_ZERO implementation into its own PR or move unrelated refactors and packaging updates into a follow-up PR; if the NULL-handling change in float_negate is intentional, document and add tests for that behavioral change, otherwise revert it before merging.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Add FLOAT_IS_ZERO database function" is concise, specific, and accurately summarizes the primary change in the PR—the addition of a new FLOAT_IS_ZERO SQLite function and its tests—so it clearly communicates the main intent to reviewers scanning history.
Linked Issues Check ✅ Passed The PR implements and registers a FLOAT_IS_ZERO SQLite function, provides an FFI entry that returns 1 for zero and 0 for non-zero while preserving NULL inputs, adds parsing and error handling for hex floats, and includes unit/integration tests covering zero, non-zero, NULL, and invalid inputs, which satisfies the linked issue #13 requirement to provide a function that indicates whether a Float is zero.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch float-is-zero

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89fb9c4 and 6f052aa.

⛔ Files ignored due to path filters (1)
  • svelte-test/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • svelte-test/tests/fixtures/test-helpers.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.309Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.
📚 Learning: 2025-09-17T08:04:44.062Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Applied to files:

  • svelte-test/tests/fixtures/test-helpers.ts
📚 Learning: 2025-09-19T07:06:37.309Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.309Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Applied to files:

  • svelte-test/tests/fixtures/test-helpers.ts
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Applied to files:

  • svelte-test/tests/fixtures/test-helpers.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (1)
svelte-test/tests/fixtures/test-helpers.ts (1)

191-192: LGTM — include float_is_zero_test in cleanup.

Adding 'float_is_zero_test' to the drop list aligns with the new FLOAT_IS_ZERO tests and prevents leakage across runs.


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

@findolor findolor linked an issue Sep 17, 2025 that may be closed by this pull request
@0xgleb 0xgleb requested review from 0xgleb and hardyjosh September 18, 2025 16:27
Base automatically changed from float-negate to main September 19, 2025 06:47
@findolor
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Sep 19, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a845bdc and 84a8df5.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • svelte-test/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (6)
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1 hunks)
  • packages/sqlite-web-core/src/database_functions/mod.rs (2 hunks)
  • pkg/package.json (1 hunks)
  • svelte-test/package.json (1 hunks)
  • svelte-test/tests/database-functions/float-is-zero.test.ts (1 hunks)
  • svelte-test/tests/fixtures/test-helpers.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
svelte-test/package.json

📄 CodeRabbit inference engine (CLAUDE.md)

In the Svelte test app, depend on the sqlite-web package via the locally packed tarball produced by npm pack

Files:

  • svelte-test/package.json
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to svelte-test/package.json : In the Svelte test app, depend on the sqlite-web package via the locally packed tarball produced by npm pack

Applied to files:

  • svelte-test/package.json
📚 Learning: 2025-09-17T08:04:44.062Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
  • svelte-test/tests/fixtures/test-helpers.ts
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
  • packages/sqlite-web-core/src/database_functions/mod.rs
📚 Learning: 2025-09-17T17:09:58.740Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
📚 Learning: 2025-08-27T05:55:40.481Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
  • packages/sqlite-web-core/src/database_functions/mod.rs
🧬 Code graph analysis (2)
svelte-test/tests/database-functions/float-is-zero.test.ts (1)
svelte-test/tests/fixtures/test-helpers.ts (3)
  • PerformanceTracker (138-168)
  • createTestDatabase (6-22)
  • cleanupDatabase (173-204)
packages/sqlite-web-core/src/database_functions/mod.rs (1)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1)
  • float_is_zero (18-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (14)
pkg/package.json (1)

15-15: No-op formatting change.

EOF newline tweak only; safe to merge.

packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1)

3-16: Helper logic is solid.

Trim, empty-check, hex-parse via Float::from_hex, and zero-eval with error mapping all look correct, including handling of negative zero via Float::is_zero().

packages/sqlite-web-core/src/database_functions/mod.rs (2)

11-11: Module import OK.

New function module wired into the registry module.


17-17: Use import OK.

Brings float_is_zero symbol into scope for registration.

svelte-test/package.json (1)

44-44: Dependency tarball matches package version — OK.
svelte-test/package.json dependency equals the expected tarball: file:../pkg/rainlanguage-sqlite-web-0.0.1-alpha.2.tgz.

svelte-test/tests/database-functions/float-is-zero.test.ts (9)

23-33: Schema choice LGTM; avoids prior NOT NULL pitfalls

Table uses value TEXT (nullable). Good—prevents the NULL insert constraint issue seen in earlier float tests.


35-37: Good: test cleanup includes table drop

Matches cleanupDatabase’s allowlist (float_is_zero_test). Prevents test cross-talk.


71-81: Zero detection assertions look correct

Covers exact zero, non-zero, duplicate zero, and trimmed input.


83-90: Filtering via FLOAT_IS_ZERO is validated correctly

Count-based assertion is clear and deterministic.


113-126: Non-zero aggregate case covered

Good contrast to the balanced case.


129-134: NULL behavior verified

Matches spec: returns SQL NULL for NULL input.


144-148: Minimize coupling to exact error text

If the Rust side doesn’t guarantee stable messages, prefer checking a stable prefix or code. If an error code is exposed, assert on it.

Do we expose structured error codes for DB functions? If yes, return them through db.query and assert on code rather than message.


60-70: Prefer parameter binding over string interpolation (if supported)

Avoids quoting pitfalls and future inputs with special chars. If db.query supports params, switch to them; otherwise keep as-is.

⛔ Skipped due to learnings
Learnt from: findolor
PR: rainlanguage/sqlite-web#15
File: svelte-test/tests/integration/error-handling.test.ts:418-434
Timestamp: 2025-09-17T17:44:54.855Z
Learning: In the sqlite-web test harness, parameter binding is not available for the db.query() method, so string interpolation with manual SQL escaping using value.replace(/'/g, "''") is the necessary approach for handling special characters in test data.
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

1-8: Verify ESM import-extension consistency for test fixtures

float-is-zero.test.ts imports ../fixtures/test-helpers.js — confirm whether other tests import the same helper with or without the .js extension and make imports consistent to avoid NodeNext/bundler resolution mismatches.

Location: svelte-test/tests/database-functions/float-is-zero.test.ts (lines 1–8)

Quick check (run locally): rg -n 'test-helpers' svelte-test/tests -S — inspect matching import lines and normalize extensions.

Comment on lines 18 to 31
pub unsafe extern "C" fn float_is_zero(
context: *mut sqlite3_context,
argc: c_int,
argv: *mut *mut sqlite3_value,
) {
if argc != 1 {
sqlite3_result_error(
context,
c"FLOAT_IS_ZERO() requires exactly 1 argument".as_ptr(),
-1,
);
return;
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Avoid nonstandard c"..." literal to prevent build issues.

c"…" is not a standard Rust literal. Use CString to ensure portability.

Apply this diff:

-        sqlite3_result_error(
-            context,
-            c"FLOAT_IS_ZERO() requires exactly 1 argument".as_ptr(),
-            -1,
-        );
+        // Use CString for a portable C string
+        let msg = CString::new("FLOAT_IS_ZERO() requires exactly 1 argument").unwrap();
+        sqlite3_result_error(context, msg.as_ptr(), -1);

Search whether c"..." is defined anywhere; if not, this will fail to compile:


🏁 Script executed:

#!/bin/bash
rg -nP '(^|[^a-zA-Z_])c"[^"]+"' -C2 || echo 'No c"..." usages found beyond this one'
rg -nP 'macro_rules!\s*c\b|proc-macro.*\bc\b' -C2 || echo 'No macro named `c` found'

Length of output: 5330


Avoid nonstandard c"..." literal — replace with CString or provide the missing macro

rg shows multiple uses of the nonstandard c"…" literal and no macro named c in the repo; this will likely break compilation.

Occurrences:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
  • packages/sqlite-web-core/src/database_functions/rain_math.rs
  • packages/sqlite-web-core/src/database_functions/float_negate.rs
  • packages/sqlite-web-core/src/database_functions/bigint_sum.rs
  • packages/sqlite-web-core/src/database_functions/float_sum.rs

Action: replace each use with a portable C string (or add/confirm a crate that documents/provides this literal). Example diff (for float_is_zero.rs):

-        sqlite3_result_error(
-            context,
-            c"FLOAT_IS_ZERO() requires exactly 1 argument".as_ptr(),
-            -1,
-        );
+        // Use CString for a portable C string
+        let msg = CString::new("FLOAT_IS_ZERO() requires exactly 1 argument").unwrap();
+        sqlite3_result_error(context, msg.as_ptr(), -1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub unsafe extern "C" fn float_is_zero(
context: *mut sqlite3_context,
argc: c_int,
argv: *mut *mut sqlite3_value,
) {
if argc != 1 {
sqlite3_result_error(
context,
c"FLOAT_IS_ZERO() requires exactly 1 argument".as_ptr(),
-1,
);
return;
}
pub unsafe extern "C" fn float_is_zero(
context: *mut sqlite3_context,
argc: c_int,
argv: *mut *mut sqlite3_value,
) {
if argc != 1 {
// Use CString for a portable C string
let msg = CString::new("FLOAT_IS_ZERO() requires exactly 1 argument").unwrap();
sqlite3_result_error(context, msg.as_ptr(), -1);
return;
}

Comment on lines 51 to 90
#[cfg(test)]
mod tests {
use super::*;
use wasm_bindgen_test::*;

fn parse_decimal_to_hex(decimal: &str) -> String {
Float::parse(decimal.to_string()).unwrap().as_hex()
}

#[wasm_bindgen_test]
fn test_float_is_zero_hex_true_for_zero() {
let zero_hex = parse_decimal_to_hex("0");
assert!(float_is_zero_hex(&zero_hex).unwrap());
}

#[wasm_bindgen_test]
fn test_float_is_zero_hex_true_for_negative_zero() {
let neg_zero_hex = parse_decimal_to_hex("-0");
assert!(float_is_zero_hex(&neg_zero_hex).unwrap());
}

#[wasm_bindgen_test]
fn test_float_is_zero_hex_false_for_non_zero() {
let non_zero_hex = parse_decimal_to_hex("1.25");
assert!(!float_is_zero_hex(&non_zero_hex).unwrap());
}

#[wasm_bindgen_test]
fn test_float_is_zero_hex_handles_whitespace() {
let zero_hex = parse_decimal_to_hex("0");
let wrapped = format!(" {zero_hex} ");
assert!(float_is_zero_hex(&wrapped).unwrap());
}

#[wasm_bindgen_test]
fn test_float_is_zero_hex_invalid_input() {
assert!(float_is_zero_hex("").is_err());
assert!(float_is_zero_hex("not_hex").is_err());
}
}
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Tests cover core cases. Add a malformed-hex whitespace-internal case?

You already cover empty, non-hex, surrounding whitespace, ±0, and nonzero. If valuable, add a case with internal whitespace to ensure parser rejects “0x 0”.

🤖 Prompt for AI Agents
In packages/sqlite-web-core/src/database_functions/float_is_zero.rs around lines
51 to 90, add a test verifying that hex strings with internal whitespace are
rejected: create a malformed hex string containing internal space (e.g. "0x 0"
or insert a space into a valid hex), call float_is_zero_hex on it, and assert
that the result is an Err; place it alongside the other wasm_bindgen_test
functions and mirror the style of the existing invalid-input test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor Understood, the existing test coverage is sufficient. Thanks for the clarification!


🧠 Learnings used
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Comment on lines +10 to +12
const ZERO_HEX = Float.parse("0").value?.asHex() as string;
const SMALL_POS_HEX = Float.parse("0.1").value?.asHex() as string;
const OTHER_POS_HEX = Float.parse("0.5").value?.asHex() as string;
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid unsafe casts on Float.parse; fail fast if parsing fails

Casting with as string can mask parse failures and inject 'undefined' into SQL. Parse once and assert.

Apply:

-const ZERO_HEX = Float.parse("0").value?.asHex() as string;
-const SMALL_POS_HEX = Float.parse("0.1").value?.asHex() as string;
-const OTHER_POS_HEX = Float.parse("0.5").value?.asHex() as string;
+const ZERO_HEX = parseHex("0");
+const SMALL_POS_HEX = parseHex("0.1");
+const OTHER_POS_HEX = parseHex("0.5");

Add helper near the imports:

function parseHex(src: string): string {
  const parsed = Float.parse(src);
  if (!parsed.value) {
    throw new Error(`Test setup failed to parse '${src}': ${parsed.error?.msg ?? "unknown error"}`);
  }
  return parsed.value.asHex();
}
🤖 Prompt for AI Agents
In svelte-test/tests/database-functions/float-is-zero.test.ts around lines 10 to
12, avoid unsafe `as string` casts on Float.parse results; add a small helper
function near the imports that calls Float.parse(src), checks parsed.value and
throws a descriptive Error if missing, then returns parsed.value.asHex();
replace the direct casts with calls to this helper (e.g. ZERO_HEX =
parseHex("0"), etc.) so the test fails fast on parse errors and never injects
undefined into SQL.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor, understood! I'll defer to your judgment on this one.

🐰 (´・ω・`)


🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Comment on lines 39 to 57
describe("Function Availability", () => {
it("should have FLOAT_IS_ZERO function available", async () => {
try {
const pragmaResult = await db.query("PRAGMA function_list");
expect(pragmaResult).toBeDefined();
const functions = JSON.parse(pragmaResult.value || "[]");
const isZeroEntry = functions.find(
(f: any) => f.name === "FLOAT_IS_ZERO",
);
expect(isZeroEntry).toBeDefined();
} catch (error) {}

const result = await db.query(
`SELECT FLOAT_IS_ZERO('${ZERO_HEX}') as is_zero`,
);
const data = JSON.parse(result.value || "[]");
expect(Array.isArray(data)).toBe(true);
expect(data[0].is_zero).toBe(1);
});
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Optional: add a tiny helper to parse query results with error checks

Reduces repetition and ensures we don’t parse JSON when .error is set.

Example helper:

async function queryJson<T = any>(sql: string): Promise<T[]> {
  const res = await db.query(sql);
  if (res.error) throw new Error(res.error.msg);
  return JSON.parse(res.value || "[]");
}

Then replace occurrences like:

const data = await queryJson(`SELECT FLOAT_IS_ZERO('${ZERO_HEX}') as is_zero`);

Also applies to: 71-76, 83-90, 108-111, 124-126, 129-148

🤖 Prompt for AI Agents
In svelte-test/tests/database-functions/float-is-zero.test.ts around lines 39 to
57 (and similarly at lines 71-76, 83-90, 108-111, 124-126, 129-148), the test
repeatedly parses db.query results without checking for .error and duplicates
JSON.parse logic; add a small helper function (e.g., queryJson) that calls
db.query(sql), throws if res.error is present, and returns JSON.parse(res.value
|| "[]"), then replace each direct db.query + JSON.parse usage with calls to
that helper to centralize error handling and reduce repetition.

🧹 Nitpick

Don’t swallow PRAGMA errors; assert or gracefully fallback

Empty catch hides failures and weakens the “availability” test. Either assert PRAGMA or explicitly treat it as optional and still verify via SELECT.

Apply:

-  it("should have FLOAT_IS_ZERO function available", async () => {
-    try {
-      const pragmaResult = await db.query("PRAGMA function_list");
-      expect(pragmaResult).toBeDefined();
-      const functions = JSON.parse(pragmaResult.value || "[]");
-      const isZeroEntry = functions.find(
-        (f: any) => f.name === "FLOAT_IS_ZERO",
-      );
-      expect(isZeroEntry).toBeDefined();
-    } catch (error) {}
-
-    const result = await db.query(
+  it("should have FLOAT_IS_ZERO function available", async () => {
+    const pragmaResult = await db.query("PRAGMA function_list");
+    if (!pragmaResult.error && pragmaResult.value) {
+      const functions = JSON.parse(pragmaResult.value || "[]");
+      expect(functions.some((f: any) => f.name === "FLOAT_IS_ZERO")).toBe(true);
+    }
+    const result = await db.query(
       `SELECT FLOAT_IS_ZERO('${ZERO_HEX}') as is_zero`,
     );
     const data = JSON.parse(result.value || "[]");
     expect(Array.isArray(data)).toBe(true);
     expect(data[0].is_zero).toBe(1);
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe("Function Availability", () => {
it("should have FLOAT_IS_ZERO function available", async () => {
try {
const pragmaResult = await db.query("PRAGMA function_list");
expect(pragmaResult).toBeDefined();
const functions = JSON.parse(pragmaResult.value || "[]");
const isZeroEntry = functions.find(
(f: any) => f.name === "FLOAT_IS_ZERO",
);
expect(isZeroEntry).toBeDefined();
} catch (error) {}
const result = await db.query(
`SELECT FLOAT_IS_ZERO('${ZERO_HEX}') as is_zero`,
);
const data = JSON.parse(result.value || "[]");
expect(Array.isArray(data)).toBe(true);
expect(data[0].is_zero).toBe(1);
});
describe("Function Availability", () => {
it("should have FLOAT_IS_ZERO function available", async () => {
const pragmaResult = await db.query("PRAGMA function_list");
if (!pragmaResult.error && pragmaResult.value) {
const functions = JSON.parse(pragmaResult.value || "[]");
expect(functions.some((f: any) => f.name === "FLOAT_IS_ZERO")).toBe(true);
}
const result = await db.query(
`SELECT FLOAT_IS_ZERO('${ZERO_HEX}') as is_zero`,
);
const data = JSON.parse(result.value || "[]");
expect(Array.isArray(data)).toBe(true);
expect(data[0].is_zero).toBe(1);
});
🤖 Prompt for AI Agents
In svelte-test/tests/database-functions/float-is-zero.test.ts around lines 39 to
57, the catch block around the PRAGMA query is empty and swallows errors;
replace it by either asserting the PRAGMA call succeeded (fail the test on
error) or explicitly treating it as optional by logging the error and continuing
to verify function existence via the SELECT fallback. Concretely: remove the
empty catch, add an assertion like expect(error).toBeUndefined() or rethrow the
error so the test fails on PRAGMA failure, or if you choose optional behavior,
catch the error and record a clear message (e.g., console.warn) before
proceeding to the SELECT-based verification and ensure the rest of the test
still properly checks the FLOAT_IS_ZERO result.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pragma test is not available in this env so removing that altogether

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor, understood! Since PRAGMA function_list isn't available in your test environment, let's remove that check entirely and rely on the direct function call for both availability and functionality verification.

Here's the simplified version:

describe("Function Availability", () => {
  it("should have FLOAT_IS_ZERO function available", async () => {
    const result = await db.query(
      `SELECT FLOAT_IS_ZERO('${ZERO_HEX}') as is_zero`,
    );
    const data = JSON.parse(result.value || "[]");
    expect(Array.isArray(data)).toBe(true);
    expect(data[0].is_zero).toBe(1);
  });

This approach is actually cleaner since the direct function call serves as both an availability test (will fail if function doesn't exist) and a basic functionality test (verifies it returns the expected result for zero).


✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:39-57
Timestamp: 2025-09-19T07:06:20.147Z
Learning: PRAGMA function_list is not available in the WASM SQLite test environment used by svelte-test, so availability testing should rely on direct function calls rather than PRAGMA queries.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Comment on lines +60 to +90
describe("Zero Detection", () => {
beforeEach(async () => {
await db.query(`
INSERT INTO float_is_zero_test (value) VALUES
('${ZERO_HEX}'),
('${SMALL_POS_HEX}'),
('${ZERO_HEX}'),
(' ${ZERO_HEX} ')
`);
});

it("should flag exact zero hex strings", async () => {
const result = await db.query(
"SELECT value, FLOAT_IS_ZERO(value) as is_zero FROM float_is_zero_test ORDER BY id",
);
const rows = JSON.parse(result.value || "[]") as ZeroRow[];
expect(rows).toHaveLength(4);
expect(rows[0].is_zero).toBe(1);
expect(rows[1].is_zero).toBe(0);
expect(rows[2].is_zero).toBe(1);
expect(rows[3].is_zero).toBe(1);
});

it("should support filtering queries", async () => {
const result = await db.query(`
SELECT COUNT(*) as zero_count FROM float_is_zero_test
WHERE FLOAT_IS_ZERO(value) = 1
`);
const data = JSON.parse(result.value || "[]");
expect(data[0].zero_count).toBe(3);
});
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Add coverage for negative zero and hex case variance

IEEE‑754 negative zero should be treated as zero. Also validate mixed‑case hex handling.

Add tests:

it("should flag negative zero hex strings", async () => {
  const NEG_ZERO_HEX = parseHex("-0");
  const r = await db.query(`SELECT FLOAT_IS_ZERO('${NEG_ZERO_HEX}') AS is_zero`);
  const d = JSON.parse(r.value || "[]");
  expect(d[0].is_zero).toBe(1);
});

it("should accept mixed-case hex digits", async () => {
  // If available in fixtures
  // import { toMixedCase } from "../fixtures/float-utils.js";
  const mixed = /* toMixedCase */ ((s: string) => s.replace(/[a-f]/g, c => (Math.random() > 0.5 ? c.toUpperCase() : c)))(ZERO_HEX);
  const r = await db.query(`SELECT FLOAT_IS_ZERO('${mixed}') AS is_zero`);
  const d = JSON.parse(r.value || "[]");
  expect(d[0].is_zero).toBe(1);
});

Also applies to: 93-127

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to worry about negative zero

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor, understood! I'll note that negative zero handling isn't required for the floating-point functions in this codebase.

The mixed-case hex test might still be valuable for robustness, but I'll defer to your judgment on whether that level of testing is necessary.

  (\   /)
 ( ._. )
o_(")(")  *noted for future reviews*

✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:39-57
Timestamp: 2025-09-19T07:06:20.147Z
Learning: PRAGMA function_list is not available in the WASM SQLite test environment used by svelte-test, so availability testing should rely on direct function calls rather than PRAGMA queries.

Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Comment on lines +93 to +111
describe("Interactions With Other Functions", () => {
it("should detect zero after negating equal values", async () => {
perf.start("float-is-zero-balanced");
const result = await db.query(`
SELECT FLOAT_IS_ZERO(total) as balanced
FROM (
SELECT FLOAT_SUM(amount) as total FROM (
SELECT '${SMALL_POS_HEX}' as amount
UNION ALL
SELECT FLOAT_NEGATE('${SMALL_POS_HEX}') as amount
)
)
`);
perf.end("float-is-zero-balanced");

const data = JSON.parse(result.value || "[]");
expect(data[0].balanced).toBe(1);
expect(perf.getDuration("float-is-zero-balanced")).toBeLessThan(1000);
});
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Make performance assertion less flaky or remove

Time-based checks are CI‑sensitive. Either drop the threshold or relax and guard against undefined.

Apply:

-      expect(data[0].balanced).toBe(1);
-      expect(perf.getDuration("float-is-zero-balanced")).toBeLessThan(1000);
+      expect(data[0].balanced).toBe(1);
+      const d = perf.getDuration("float-is-zero-balanced");
+      expect(typeof d).toBe("number");
+      expect(d!).toBeLessThan(3000);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe("Interactions With Other Functions", () => {
it("should detect zero after negating equal values", async () => {
perf.start("float-is-zero-balanced");
const result = await db.query(`
SELECT FLOAT_IS_ZERO(total) as balanced
FROM (
SELECT FLOAT_SUM(amount) as total FROM (
SELECT '${SMALL_POS_HEX}' as amount
UNION ALL
SELECT FLOAT_NEGATE('${SMALL_POS_HEX}') as amount
)
)
`);
perf.end("float-is-zero-balanced");
const data = JSON.parse(result.value || "[]");
expect(data[0].balanced).toBe(1);
expect(perf.getDuration("float-is-zero-balanced")).toBeLessThan(1000);
});
describe("Interactions With Other Functions", () => {
it("should detect zero after negating equal values", async () => {
perf.start("float-is-zero-balanced");
const result = await db.query(`
SELECT FLOAT_IS_ZERO(total) as balanced
FROM (
SELECT FLOAT_SUM(amount) as total FROM (
SELECT '${SMALL_POS_HEX}' as amount
UNION ALL
SELECT FLOAT_NEGATE('${SMALL_POS_HEX}') as amount
)
)
`);
perf.end("float-is-zero-balanced");
const data = JSON.parse(result.value || "[]");
expect(data[0].balanced).toBe(1);
const d = perf.getDuration("float-is-zero-balanced");
expect(typeof d).toBe("number");
expect(d!).toBeLessThan(3000);
});
🤖 Prompt for AI Agents
In svelte-test/tests/database-functions/float-is-zero.test.ts around lines 93 to
111, the time-based assertion
perf.getDuration("float-is-zero-balanced")).toBeLessThan(1000) is flaky on CI;
relax or guard it: replace the strict toBeLessThan(1000) with a guarded check
that handles undefined and uses a higher threshold (for example assert duration
is defined and <= 5000 or use expect(duration ??
Infinity).toBeLessThanOrEqual(5000)), or remove the timing assertion
entirely—make the test resilient by ensuring it won’t fail when perf.getDuration
returns undefined or when CI runs slower.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor Understood! I'll skip the performance assertion changes you mentioned aren't needed.

٩(◕‿◕)۶


🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:39-57
Timestamp: 2025-09-19T07:06:20.147Z
Learning: PRAGMA function_list is not available in the WASM SQLite test environment used by svelte-test, so availability testing should rely on direct function calls rather than PRAGMA queries.

Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (1)
packages/sqlite-web-core/src/database_functions/mod.rs (1)

25-43: Optional: mark RAIN_MATH_PROCESS deterministic/innocuous if pure

If it’s side‑effect‑free, add the same flags to help query planning/security posture.

-            SQLITE_UTF8,
+            SQLITE_UTF8 | SQLITE_DETERMINISTIC | SQLITE_INNOCUOUS,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84a8df5 and e132200.

⛔ Files ignored due to path filters (1)
  • svelte-test/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • packages/sqlite-web-core/src/database_functions/bigint_sum.rs (3 hunks)
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1 hunks)
  • packages/sqlite-web-core/src/database_functions/float_negate.rs (5 hunks)
  • packages/sqlite-web-core/src/database_functions/float_sum.rs (3 hunks)
  • packages/sqlite-web-core/src/database_functions/mod.rs (2 hunks)
  • packages/sqlite-web-core/src/database_functions/rain_math.rs (2 hunks)
  • svelte-test/tests/database-functions/float-is-zero.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.
📚 Learning: 2025-09-17T08:04:44.062Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-19T07:06:37.278Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
  • packages/sqlite-web-core/src/database_functions/mod.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-19T07:06:20.147Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:39-57
Timestamp: 2025-09-19T07:06:20.147Z
Learning: PRAGMA function_list is not available in the WASM SQLite test environment used by svelte-test, so availability testing should rely on direct function calls rather than PRAGMA queries.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
  • packages/sqlite-web-core/src/database_functions/rain_math.rs
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
  • packages/sqlite-web-core/src/database_functions/float_sum.rs
  • packages/sqlite-web-core/src/database_functions/float_negate.rs
  • packages/sqlite-web-core/src/database_functions/bigint_sum.rs
  • packages/sqlite-web-core/src/database_functions/mod.rs
📚 Learning: 2025-09-17T17:09:58.740Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.

Applied to files:

  • svelte-test/tests/database-functions/float-is-zero.test.ts
📚 Learning: 2025-09-17T18:08:06.145Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: packages/sqlite-web-core/src/database_functions/float_negate.rs:16-17
Timestamp: 2025-09-17T18:08:06.145Z
Learning: The rain-math-float Float type implements arithmetic operations (including unary negation with the - operator) to return Result types rather than direct values, requiring .map_err(...)? for error handling of potential mathematical errors like overflow or underflow.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/rain_math.rs
  • packages/sqlite-web-core/src/database_functions/float_negate.rs
📚 Learning: 2025-08-27T05:55:40.481Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
  • packages/sqlite-web-core/src/database_functions/float_sum.rs
  • packages/sqlite-web-core/src/database_functions/bigint_sum.rs
  • packages/sqlite-web-core/src/database_functions/mod.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/messages.rs : Use structured message types for worker communication as defined in messages.rs

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_sum.rs
🧬 Code graph analysis (2)
svelte-test/tests/database-functions/float-is-zero.test.ts (1)
svelte-test/tests/fixtures/test-helpers.ts (3)
  • PerformanceTracker (138-168)
  • createTestDatabase (6-22)
  • cleanupDatabase (173-204)
packages/sqlite-web-core/src/database_functions/mod.rs (1)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1)
  • float_is_zero (20-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (8)
packages/sqlite-web-core/src/database_functions/rain_math.rs (1)

3-4: Good: centralized, NUL‑terminated error message constant

Switching to a module‑level byte slice and reusing its pointer is correct and consistent with other modules.

Also applies to: 14-15

packages/sqlite-web-core/src/database_functions/float_sum.rs (1)

3-5: Good: constantized error strings; behavior unchanged

Using private, NUL‑terminated constants removes the nonstandard literal usage and keeps FFI happy.

Also applies to: 52-53, 72-73

packages/sqlite-web-core/src/database_functions/bigint_sum.rs (1)

3-5: Good: consistent error‑message constants

Matches the pattern used elsewhere; no behavior change.

Also applies to: 63-63, 83-83

packages/sqlite-web-core/src/database_functions/float_negate.rs (1)

3-7: Good: explicit UTF‑8 handling and constantized errors

Early NULL check + to_str() path with clear error constants is solid. Return string uses SQLITE_TRANSIENT helper; OK.

Also applies to: 37-38, 56-60, 76-79, 89-90

packages/sqlite-web-core/src/database_functions/float_is_zero.rs (2)

5-18: Helper logic is clear and precise

Trim + parse + is_zero() with Result propagation is straightforward.


25-28: OK: argc check and error path

The FFI guard is correct.

packages/sqlite-web-core/src/database_functions/mod.rs (1)

11-11: Registration looks correct; flags match FLOAT_NEGATE

FLOAT_IS_ZERO is registered with SQLITE_UTF8 | SQLITE_DETERMINISTIC | SQLITE_INNOCUOUS; good.

Also applies to: 17-18, 105-123

svelte-test/tests/database-functions/float-is-zero.test.ts (1)

39-47: Tests read well and cover core paths

Availability via SELECT, zero/non‑zero, whitespace, NULL, and error cases are all exercised.

Also applies to: 50-81, 83-117, 119-147

Comment on lines +36 to +38
let value_str = CStr::from_ptr(value_ptr as *const c_char).to_string_lossy();

match float_is_zero_hex(&value_str) {
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid lossy UTF‑8; mirror FLOAT_NEGATE’s stricter handling

Use CStr::to_str() and a dedicated invalid‑UTF8 error constant to avoid silent replacement of bytes.

-    let value_str = CStr::from_ptr(value_ptr as *const c_char).to_string_lossy();
+    let value_cstr = CStr::from_ptr(value_ptr as *const c_char);
+    let value_str = match value_cstr.to_str() {
+        Ok(s) => s,
+        Err(_) => {
+            sqlite3_result_error(
+                context,
+                FLOAT_IS_ZERO_INVALID_UTF8_MESSAGE.as_ptr() as *const c_char,
+                -1,
+            );
+            return;
+        }
+    };

Add at top with other constants:

+const FLOAT_IS_ZERO_INVALID_UTF8_MESSAGE: &[u8] = b"invalid UTF-8\0";

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/sqlite-web-core/src/database_functions/float_is_zero.rs around lines
36 to 38, replace the lossy CStr::to_string_lossy() call with CStr::to_str() and
mirror FLOAT_NEGATE’s stricter handling by adding an INVALID_UTF8 constant at
the top with the other constants; call to_str(), map Err(_) to return or
propagate that INVALID_UTF8 error constant (instead of silently replacing
bytes), and use the resulting &str with float_is_zero_hex so invalid UTF‑8 is
handled consistently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not an issue

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor Understood, I'll defer to your judgment on this. The current UTF-8 handling approach works fine for your use case.


🧠 Learnings used
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Comment on lines +42 to +45
Err(e) => {
let error_msg = format!("{e}\0");
sqlite3_result_error(context, error_msg.as_ptr() as *const c_char, -1);
}
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use CString for error propagation; guard interior NUL

Align with FLOAT_NEGATE to avoid truncated messages if error text contains NULs.

-            let error_msg = format!("{e}\0");
-            sqlite3_result_error(context, error_msg.as_ptr() as *const c_char, -1);
+            match CString::new(e) {
+                Ok(msg) => sqlite3_result_error(context, msg.as_ptr(), -1),
+                Err(_) => {
+                    sqlite3_result_error(
+                        context,
+                        FLOAT_IS_ZERO_ERROR_MESSAGE_INTERIOR_NUL.as_ptr() as *const c_char,
+                        -1,
+                    );
+                }
+            }

Add constant:

+const FLOAT_IS_ZERO_ERROR_MESSAGE_INTERIOR_NUL: &[u8] =
+    b"Error message contained interior NUL\0";

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/sqlite-web-core/src/database_functions/float_is_zero.rs around lines
42 to 45, the error path formats a Rust string with an embedded NUL and passes a
raw pointer to sqlite3_result_error, which can truncate messages or be unsafe;
change this to create a CString (guarding against interior NULs) and pass its
pointer to sqlite3_result_error, matching the approach used in FLOAT_NEGATE.
Also add the suggested constant (e.g., for SQLITE_TRANSIENT or a length
constant) in the same module if required by FLOAT_NEGATE for consistent
handling.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor Understood! I'll note your decision that the CString error handling pattern is not needed for the FLOAT_IS_ZERO function. Thank you for the clarification.


🧠 Learnings used
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1)

46-49: Prefer CString for error propagation (repeat).

Reiterating prior note: formatting a Rust String with a trailing NUL and passing .as_ptr() risks truncation on interior NULs. If you want consistency with other fns, consider CString::new(e) and fall back to a static message on NUL errors. If you’re intentionally keeping it minimal, feel free to ignore.

Apply if desired:

-        Err(e) => {
-            let error_msg = format!("{e}\0");
-            sqlite3_result_error(context, error_msg.as_ptr() as *const c_char, -1);
-        }
+        Err(e) => {
+            if let Ok(msg) = CString::new(e) {
+                sqlite3_result_error(context, msg.as_ptr(), -1);
+            } else {
+                static MSG: &[u8] = b"FLOAT_IS_ZERO: error message contained interior NUL\0";
+                sqlite3_result_error(context, MSG.as_ptr() as *const c_char, -1);
+            }
+        }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e132200 and dbe1e99.

📒 Files selected for processing (1)
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-19T07:06:37.278Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-08-27T05:55:40.481Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/worker_template.rs : Ensure the worker JavaScript generated by worker_template.rs embeds the WASM (base64) and does not fetch external WASM files

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T08:04:44.062Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (6)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (6)

5-18: Core helper is correct and tight.

Trims input, validates empty, parses via Float::from_hex, and defers zero-check — good separation and error messages.


25-33: Good: defensive argc check with clear message.

Consistent with other DB fns and avoids undefined argv access.


34-38: NULL propagation is correct.

Returning SQL NULL for NULL input matches prior functions’ behavior.


42-45: Deterministic boolean mapping looks good.

Returning 1/0 is SQLite‑idiomatic for booleans.


3-3: Naming matches module pattern.

FLOAT_IS_ZERO_ARG_ERROR_MESSAGE aligns with existing conventions. Thanks for making this consistent.


1-1: Confirmed: FLOAT_IS_ZERO registered with SQLITE_UTF8 | SQLITE_DETERMINISTIC | SQLITE_INNOCUOUS and exported.
float_is_zero is defined at packages/sqlite-web-core/src/database_functions/float_is_zero.rs:20 and registered in packages/sqlite-web-core/src/database_functions/mod.rs (lines 110–114) with SQLITE_UTF8 | SQLITE_DETERMINISTIC | SQLITE_INNOCUOUS and Some(float_is_zero) as the xFunc.

Comment on lines +20 to +31
pub unsafe extern "C" fn float_is_zero(
context: *mut sqlite3_context,
argc: c_int,
argv: *mut *mut sqlite3_value,
) {
if argc != 1 {
sqlite3_result_error(
context,
FLOAT_IS_ZERO_ARG_ERROR_MESSAGE.as_ptr() as *const c_char,
-1,
);
return;
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Small robustness nits (non‑blocking).

  • argv is assumed non‑NULL when argc > 0. If you want belt‑and‑suspenders, guard argv.is_null() and bail early.
  • Using to_string_lossy() is acceptable here; no change requested.

Also applies to: 34-41

🤖 Prompt for AI Agents
In packages/sqlite-web-core/src/database_functions/float_is_zero.rs around lines
20-31 (and also apply the same change to lines 34-41), the code assumes argv is
non-null when argc > 0; add an explicit check for argv.is_null() and return an
sqlite error if it is null (mirror the existing argc != 1 error handling) before
dereferencing argv, keeping the rest of the logic unchanged and retaining
to_string_lossy() usage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor, understood! I'll note that the argv null check isn't needed in this context. Thanks for the clarification.

    (\   /)
   ( ._.)
  o_(")(")

🧠 Learnings used
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbe1e99 and 1a54ded.

📒 Files selected for processing (1)
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:64-72
Timestamp: 2025-09-17T17:09:58.740Z
Learning: FLOAT_SUM functionality is available and working in the codebase, so tests can rely on it without needing availability checks or gating logic.
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-19T07:06:37.278Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-08-27T05:55:40.481Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/worker_template.rs : Ensure the worker JavaScript generated by worker_template.rs embeds the WASM (base64) and does not fetch external WASM files

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T08:04:44.062Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (6)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (6)

3-3: Constant looks correct and consistent.

NUL-terminated static message is appropriate for sqlite3_result_error.


5-18: Helper is tight and well-scoped.

Trim + explicit empty check + precise error mapping reads well.


25-32: Good argc guard.

Clear error path on wrong arity; matches patterns in sibling modules.


42-49: Error propagation path is fine.

Message is surfaced to SQLite; control flow is clear.


53-86: Tests cover the core contract.

Zero/non‑zero, whitespace, and invalid inputs are exercised.


20-24: Confirmed: FLOAT_IS_ZERO is registered with arity=1 and flags SQLITE_UTF8 | SQLITE_DETERMINISTIC | SQLITE_INNOCUOUS in packages/sqlite-web-core/src/database_functions/mod.rs. No action required.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (2)

45-45: Lossy UTF‑8 conversion accepted (per prior discussion).
Acknowledging the earlier decision to keep to_string_lossy here.


52-54: Interior NUL risk in error string noted previously; proceeding as decided.
Current approach is acceptable given project stance.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a54ded and c57e46f.

📒 Files selected for processing (1)
  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-19T07:06:37.278Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-08-27T05:55:40.481Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/worker_template.rs : Ensure the worker JavaScript generated by worker_template.rs embeds the WASM (base64) and does not fetch external WASM files

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T08:04:44.062Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies characters matching /[a-f]/ (hex digits a-f), preserving the 'x' in '0x' prefixes and all other characters unchanged, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
📚 Learning: 2025-09-17T17:17:41.827Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: svelte-test/tests/database-functions/float-negate.test.ts:105-119
Timestamp: 2025-09-17T17:17:41.827Z
Learning: The toMixedCase function in svelte-test/tests/fixtures/float-utils.ts only modifies [a-f] characters, preserving the 'x' in '0x' prefixes, so it won't create conflicts with tests that reject '0X' prefixes.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/float_is_zero.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (8)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (8)

3-3: Consistent, NUL‑terminated arg error constant — good.
Static, NUL‑terminated bytes avoid lifetime issues and match the crate’s pattern.


5-18: Helper trims, validates, and delegates cleanly.
Clear errors, no surprises. All good.


20-33: Arg‑count guard is correct and portable.
Uses a stable, NUL‑terminated message with sqlite3_result_error(-1).


34-43: Nice: distinguishes SQL NULL vs OOM from sqlite3_value_text.
This mirrors SQLite semantics and avoids misreporting OOM as NULL.


63-66: Test helper looks good.
Matches existing Float parse→hex pattern.


67-84: Good coverage: zero, non‑zero, surrounding whitespace.
Behavior verified along critical paths.


86-90: Invalid‑input tests are appropriate.
Ensures empty and non‑hex are rejected.


20-24: Verify registration wiring and external tests reference

rg returned no matches — confirm FLOAT_IS_ZERO is registered (arity=1, deterministic+innocuous) via create_function/create_function_v2, exported from the core module, and exercised by svelte-test.

packages/sqlite-web-core/src/database_functions/float_is_zero.rs (fn at lines 20–24)


let value_str = CStr::from_ptr(value_ptr as *const c_char).to_string_lossy();

match float_is_zero_hex(&value_str) {
Copy link

@coderabbitai coderabbitai bot Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Nit: make the deref explicit for readability.
Use as_ref() to pass &str explicitly from Cow.

-    match float_is_zero_hex(&value_str) {
+    match float_is_zero_hex(value_str.as_ref()) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
match float_is_zero_hex(&value_str) {
match float_is_zero_hex(value_str.as_ref()) {
🤖 Prompt for AI Agents
In packages/sqlite-web-core/src/database_functions/float_is_zero.rs around line
47, the call currently passes &value_str where value_str is a Cow<str>; make the
dereference explicit for readability by calling value_str.as_ref() (i.e., pass
the &str via as_ref()) when invoking float_is_zero_hex so the function receives
an explicit &str instead of relying on implicit deref coercion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@findolor, understood! You're absolutely right - the implicit deref coercion from &Cow<str> to &str works perfectly fine here. The current code is clear and functional as-is.

ฅ(•ㅅ•❀)ฅ


🧠 Learnings used
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.278Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/worker_template.rs : Ensure the worker JavaScript generated by worker_template.rs embeds the WASM (base64) and does not fetch external WASM files

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c57e46f and 89fb9c4.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • svelte-test/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • packages/sqlite-web-core/src/database_functions/mod.rs (2 hunks)
  • pkg/package.json (1 hunks)
  • svelte-test/package.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
svelte-test/package.json

📄 CodeRabbit inference engine (CLAUDE.md)

In the Svelte test app, depend on the sqlite-web package via the locally packed tarball produced by npm pack

Files:

  • svelte-test/package.json
🧠 Learnings (6)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.309Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Learnt from: findolor
PR: rainlanguage/sqlite-web#6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to svelte-test/package.json : In the Svelte test app, depend on the sqlite-web package via the locally packed tarball produced by npm pack

Applied to files:

  • svelte-test/package.json
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
PR: rainlanguage/sqlite-web#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries

Applied to files:

  • packages/sqlite-web-core/src/database_functions/mod.rs
📚 Learning: 2025-09-19T07:06:37.309Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:60-90
Timestamp: 2025-09-19T07:06:37.309Z
Learning: For floating-point database functions in the rainlanguage/sqlite-web codebase, negative zero handling is not required and doesn't need to be tested.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/mod.rs
📚 Learning: 2025-08-27T05:55:40.481Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#5
File: packages/sqlite-web-core/src/database_functions/bigint_sum.rs:76-93
Timestamp: 2025-08-27T05:55:40.481Z
Learning: In the sqlite-web-core codebase, for SQLite aggregate function context initialization, the maintainer prefers using byte-scanning to detect zero-initialized memory rather than using explicit initialization flags or Option wrappers.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/mod.rs
📚 Learning: 2025-09-17T18:08:06.145Z
Learnt from: findolor
PR: rainlanguage/sqlite-web#11
File: packages/sqlite-web-core/src/database_functions/float_negate.rs:16-17
Timestamp: 2025-09-17T18:08:06.145Z
Learning: The rain-math-float Float type implements arithmetic operations (including unary negation with the - operator) to return Result types rather than direct values, requiring .map_err(...)? for error handling of potential mathematical errors like overflow or underflow.

Applied to files:

  • packages/sqlite-web-core/src/database_functions/mod.rs
🧬 Code graph analysis (1)
packages/sqlite-web-core/src/database_functions/mod.rs (1)
packages/sqlite-web-core/src/database_functions/float_is_zero.rs (1)
  • float_is_zero (20-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (4)
pkg/package.json (1)

15-15: LGTM—no semantic change.

Trailing newline addition only; package metadata unaffected.

packages/sqlite-web-core/src/database_functions/mod.rs (3)

11-11: Module added correctly.

mod float_is_zero; follows existing module pattern.


18-18: Import is appropriate.

Bringing float_is_zero::* into scope is consistent with nearby modules.


127-146: Registration looks correct and secure.

FLOAT_IS_ZERO is registered with 1 arg and SQLITE_DETERMINISTIC | SQLITE_INNOCUOUS; matches FLOAT_NEGATE and resolves the earlier nit.

Comment on lines +127 to +146
// Register FLOAT_IS_ZERO scalar function
let float_is_zero_name = CString::new("FLOAT_IS_ZERO").unwrap();
let ret = unsafe {
sqlite3_create_function_v2(
db,
float_is_zero_name.as_ptr(),
1, // 1 argument
SQLITE_UTF8 | SQLITE_DETERMINISTIC | SQLITE_INNOCUOUS,
std::ptr::null_mut(),
Some(float_is_zero), // xFunc for scalar
None, // No xStep
None, // No xFinal
None, // No destructor
)
};

if ret != SQLITE_OK {
return Err("Failed to register FLOAT_IS_ZERO function".to_string());
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Consider DRYing up function registration.

You now have several near-identical sqlite3_create_function_v2 calls; a small helper (e.g., register_scalar(db, "NAME", nargs, flags, xFunc)) would reduce repetition. Non-blocking.

🤖 Prompt for AI Agents
In packages/sqlite-web-core/src/database_functions/mod.rs around lines 127 to
146, several near-identical sqlite3_create_function_v2 calls are duplicated
(registering scalar functions like FLOAT_IS_ZERO); create a small helper (e.g.,
register_scalar) that accepts parameters (db, name: &str, nargs: c_int, flags:
i32, xfunc: Option<...>) to build the CString, call sqlite3_create_function_v2,
check the return value and return a Result<(), String>, then replace each
repeated block with calls to this helper to reduce duplication and centralize
error reporting.

"dependencies": {
"@rainlanguage/float": "^0.0.0-alpha.22",
"@rainlanguage/sqlite-web": "file:../pkg/rainlanguage-sqlite-web-0.0.1-alpha.3.tgz"
"@rainlanguage/sqlite-web": "file:../pkg/rainlanguage-sqlite-web-0.0.1-alpha.4.tgz"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

LGTM; ensure the tarball exists and CI packs it before install

The local tarball bump to alpha.4 aligns with the guideline to depend via a locally packed tarball. Please verify the version–filename match and presence of the tarball, and ensure CI runs npm pack for @rainlanguage/sqlite-web before installing svelte-test.

Run this check from repo root:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

S_PKG="svelte-test/package.json"

dep=$(jq -r '.dependencies["@rainlanguage/sqlite-web"]' "$S_PKG")
echo "Dependency spec: $dep"

[[ "$dep" == file:../pkg/* ]] || { echo "ERROR: Not a ../pkg tarball"; exit 1; }
tarball="${dep#file:../pkg/}"

pkgjson=$(fd -H -a package.json -E node_modules | while read -r f; do
  name=$(jq -r '.name // ""' "$f")
  if [[ "$name" == "@rainlanguage/sqlite-web" ]]; then echo "$f"; break; fi
done)

[[ -n "${pkgjson:-}" ]] || { echo "ERROR: @rainlanguage/sqlite-web/package.json not found"; exit 1; }

version=$(jq -r '.version' "$pkgjson")
expected="rainlanguage-sqlite-web-$version.tgz"

[[ "$tarball" == "$expected" ]] || { echo "ERROR: Tarball mismatch: have $tarball, expected $expected"; exit 1; }
[[ -f "pkg/$expected" ]] || { echo "ERROR: Missing pkg/$expected. Run: npm --workspace=@rainlanguage/sqlite-web pack --pack-destination ./pkg"; exit 1; }

echo "OK: Tarball matches version and exists at pkg/$expected"

Length of output: 208


Missing tarball: svelte-test/package.json references ../pkg/rainlanguage-sqlite-web-0.0.1-alpha.4.tgz but pkg/rainlanguage-sqlite-web-0.0.1-alpha.4.tgz is absent

svelte-test/package.json (line 44) must point to an existing tarball — run npm --workspace=@rainlanguage/sqlite-web pack --pack-destination ./pkg in CI before installing svelte-test, or update the dependency to an existing tarball/version.

🤖 Prompt for AI Agents
In svelte-test/package.json at line 44 the dependency points to a non-existent
local tarball ../pkg/rainlanguage-sqlite-web-0.0.1-alpha.4.tgz; fix by either
running `npm --workspace=@rainlanguage/sqlite-web pack --pack-destination ./pkg`
(or the equivalent CI step) to produce that tarball before installing
svelte-test, or update the package.json to reference an existing tarball path or
a published version that exists.

@findolor findolor merged commit 7601e4c into main Sep 23, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add FLOAT_IS_ZERO database function

4 participants