-
Notifications
You must be signed in to change notification settings - Fork 0
Add FLOAT_IS_ZERO database function
#14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds 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
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-09-17T08:04:44.062ZApplied to files:
📚 Learning: 2025-09-19T07:06:37.309ZApplied to files:
📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this 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
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locksvelte-test/package-lock.jsonis 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.tssvelte-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.tspackages/sqlite-web-core/src/database_functions/float_is_zero.rspackages/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.rspackages/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 pitfallsTable uses
value TEXT(nullable). Good—prevents the NULL insert constraint issue seen in earlier float tests.
35-37: Good: test cleanup includes table dropMatches cleanupDatabase’s allowlist (float_is_zero_test). Prevents test cross-talk.
71-81: Zero detection assertions look correctCovers exact zero, non-zero, duplicate zero, and trimmed input.
83-90: Filtering via FLOAT_IS_ZERO is validated correctlyCount-based assertion is clear and deterministic.
113-126: Non-zero aggregate case coveredGood contrast to the balanced case.
129-134: NULL behavior verifiedMatches spec: returns SQL NULL for NULL input.
144-148: Minimize coupling to exact error textIf 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.queryand 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.querysupports 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 fixturesfloat-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.
| 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; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
| 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; | |
| } |
| #[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()); | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed
There was a problem hiding this comment.
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.
| 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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fine
There was a problem hiding this comment.
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
| 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); | ||
| }); |
There was a problem hiding this comment.
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.
| 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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| 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); | ||
| }); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| 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); | ||
| }); |
There was a problem hiding this comment.
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.
| 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed
There was a problem hiding this comment.
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.
There was a problem hiding this 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 pureIf 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
⛔ Files ignored due to path filters (1)
svelte-test/package-lock.jsonis 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.tspackages/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.tspackages/sqlite-web-core/src/database_functions/float_is_zero.rspackages/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.tspackages/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.tspackages/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.tspackages/sqlite-web-core/src/database_functions/rain_math.rspackages/sqlite-web-core/src/database_functions/float_is_zero.rspackages/sqlite-web-core/src/database_functions/float_sum.rspackages/sqlite-web-core/src/database_functions/float_negate.rspackages/sqlite-web-core/src/database_functions/bigint_sum.rspackages/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.rspackages/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.rspackages/sqlite-web-core/src/database_functions/float_sum.rspackages/sqlite-web-core/src/database_functions/bigint_sum.rspackages/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 constantSwitching 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 unchangedUsing 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 constantsMatches 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 errorsEarly 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 preciseTrim + parse + is_zero() with Result propagation is straightforward.
25-28: OK: argc check and error pathThe FFI guard is correct.
packages/sqlite-web-core/src/database_functions/mod.rs (1)
11-11: Registration looks correct; flags match FLOAT_NEGATEFLOAT_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 pathsAvailability via SELECT, zero/non‑zero, whitespace, NULL, and error cases are all exercised.
Also applies to: 50-81, 83-117, 119-147
packages/sqlite-web-core/src/database_functions/float_is_zero.rs
Outdated
Show resolved
Hide resolved
| let value_str = CStr::from_ptr(value_ptr as *const c_char).to_string_lossy(); | ||
|
|
||
| match float_is_zero_hex(&value_str) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| Err(e) => { | ||
| let error_msg = format!("{e}\0"); | ||
| sqlite3_result_error(context, error_msg.as_ptr() as *const c_char, -1); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed
There was a problem hiding this comment.
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.
There was a problem hiding this 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
📒 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.
| 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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed
There was a problem hiding this comment.
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.
packages/sqlite-web-core/src/database_functions/float_is_zero.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
📒 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.
There was a problem hiding this 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
📒 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 referencerg 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) { |
There was a problem hiding this comment.
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.
| 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed
There was a problem hiding this comment.
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
There was a problem hiding this 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
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locksvelte-test/package-lock.jsonis 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_ZEROis registered with 1 arg andSQLITE_DETERMINISTIC | SQLITE_INNOCUOUS; matchesFLOAT_NEGATEand resolves the earlier nit.
| // 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()); | ||
| } | ||
|
|
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Motivation
See issue: #13
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
fix #13
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores