8000 feat(browser): BrowserDb.registerPredicate / registerAggregate (#180, PR 7) by adityamukho · Pull Request #757 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content

feat(browser): BrowserDb.registerPredicate / registerAggregate (#180, PR 7) - #757

Merged
adityamukho merged 3 commits into
v3from
feat/180-browser-udfs
Oct 25, 2026
Merged

adityamukho merged 3 commits into
v3from
feat/180-browser-udfs

Conversation

@adityamukho
Copy link
Copy Markdown
Collaborator

Part of #180 (PR 7 of the plan, steps 21; step 22 is minigraf-wasm's pin after this merges). Docs: project-minigraf/minigraf-docs (PR linked below), merge after this.

What

  • BrowserDb.registerPredicate(name, fn) and BrowserDb.registerAggregate(name, { init, step, finish }) (src/browser/udfs.rs), synchronous.
  • Values reach JS as { type, value } objects, Node's FactValue shape (integer → bigint, ref → UUID string, set → array of these). finish's result is read by binds.rs as a bind value (plain or typed form).
  • A throw (message, else String(e)), a non-boolean predicate result, or a finish result that is not a value → QRY-014.
  • udf_aggregate / udf_predicate move from db.rs into functions.rs, shared by Minigraf::register_* and BrowserDb.
  • binds.rs: read_bind_value takes the full label (slot $x) so a UDF result can reuse it; bind error messages are unchanged.

Deviation from the spec (§6.2 updated in this PR)

The spec had an unsafe impl Send + Sync wrapper over js_sys::Function. The crate is #![forbid(unsafe_code)], so instead the registry holds no JS value: functions and live accumulators sit in a thread_local! table keyed by an id unique across threads, and the closures/accumulators hold only the id (Held, removing its entry on drop). A call on a thread whose table lacks the id fails with called off the JavaScript thread, as in Node. No cfg(atomics) gating needed.

borrow_mut audit

grep -n borrow_mut src/browser/mod.rs: checkpoint (async), import_graph (async), apply_write (async, under execute), and one test. No synchronous method takes borrow_mut, so from inside a UDF query, prepare, exportGraph and register* run at once; execute/checkpoint/importGraph start from a microtask after the outer query. Tests cover a nested query, a registration, and execute + checkpoint and importGraph started inside a UDF.

Tests

15 new #[wasm_bindgen_test]s; wasm-pack test --headless --firefox --features browser: 58 passed locally (Chrome in CI). cargo clippy --all-features and wasm32 clippy clean; cargo test --test udf_test 14/14. Codex review: two findings, both checked against the code and not reproduced (the failed-registration leak case now has an assertion).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Urda7FD9CYYoxdeGGJa11x

adityamukho and others added 2 commits October 25, 2026 10:11
… PR 7)

JS user functions in BrowserDb. Values reach JS as {type, value} objects
(Node's FactValue shape); finish's result is read as a bind value. A throw,
a non-boolean predicate result or a bad finish result fails the query with
QRY-014. Re-entry: query/prepare/register* run synchronously from a UDF;
execute/checkpoint/importGraph run after the outer query.

The crate forbids unsafe, so the registry holds ids into a thread-local
table of JS values instead of an unsafe Send + Sync wrapper; spec §6.2
updated. udf_aggregate/udf_predicate move to functions.rs, shared with
Minigraf's register methods.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Urda7FD9CYYoxdeGGJa11x
@codecov
codecov Bot commented Oct 25, 2026
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/query/datalog/functions.rs 93.93% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@adityamukho
adityamukho merged commit 02243f1 into v3 Oct 25, 2026
19 checks passed
@adityamukho
adityamukho deleted the feat/180-browser-udfs branch October 25, 2026 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

0