🚧 fix: bridge scoped-builtin sub-reasoner calls to nested WASM instances - #1956
Draft
jeswr wants to merge 1 commit into
Draft
🚧 fix: bridge scoped-builtin sub-reasoner calls to nested WASM instances#1956jeswr wants to merge 1 commit into
jeswr wants to merge 1 commit into
Conversation
The graph-scoped builtins (log:collectAllIn, log:forAllIn, log:ifThenElseIn, log:conclusion, log:satisfiable, e:call, e:fail, e:findall) evaluate their scope graph by spawning a fresh eye process through exec/2 -> shell/2, which is unavailable under Emscripten, so they all failed in eye-js. Generate the pvm with an emscripten-conditional redefinition of exec/2 that yields the sub-reasoner command line to the JavaScript host with await/2 (the mechanism already used for log:ask), and answer those yields by booting a fresh swipl-wasm module with the same image - the isolation analogue of the process spawn. The bridge copies the temporary input files into the fresh module, runs the sub-query, writes the captured stdout to the redirect target of the requesting instance, and reports the recorded exit code. Sub-reasoners get the same driver recursively, so scoped builtins nested inside scope graphs work too, and the two identical invocations EYE issues per scope evaluation are answered from a content-keyed cache. e:exec (arbitrary shell commands) remains unsupported and now fails with a descriptive error. Closes #873 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #873
What
All graph-literal-scoped builtins failed in eye-js because they are evaluated through one helper in eye.pl,
exec/2, which spawns a fresheyeprocess (shell/2) to compute the deductive closure of the scope graph in isolation — and there is no shell under Emscripten. This PR bridges those sub-reasoner invocations to fresh swipl-wasm module instances instead, following the design in this investigation comment on #873.Covered builtins (everything that funnels through
exec/2):log:collectAllIn,log:forAllIn,log:ifThenElseIn,log:conclusion,log:satisfiable,e:call,e:fail,e:findall. The ninthexec/2caller,e:exec(arbitrary shell commands), stays unsupported and now fails with a descriptive error naming this issue instead ofshell/2: Function not implemented.On the exact repro from #873,
n3reasonernow returns(:x) a :Result.— identical to native EYE.How
scripts/generate-pvm.ts): the fetchedeye.plgets an emscripten-conditional snippet appended beforeqsave_programthat redefinesexec/2to yield the command line to the host viaawait/2— the same mechanismeye.plalready uses forlog:ask(userInput/2). Since the image is generated by running the source inside swipl-wasm, the conditional is live at consult time and the redefinition is compiled into the saved state; native builds are untouched.lingua.pldefines noexec/2and is left as-is.lib/bridge.ts, new):bridgeCallback(module, { spawn, cb })produces a callback for the existing async yield loop (qaQuery). On aneyejs:execyield it parses the command line (eye --nope [--quiet] Data [--query Query | --pass-all] > Out), boots a fresh swipl-wasm module with the same image — the isolation analogue of the process spawn, preserving the scoped semantics (the sub-run must see only the scope graph, and EYE's KB/flags/globals are instance-wide) — copies the temp files across the two MEMFS instances, runs the sub-query, writes the captured stdout to the redirect target in the requesting instance's FS, and resumes with"ok". Failures surface as the recorded exit code (eye.pl'smain/1trapshalt/1into theexit_codeglobal), so e.g. an inference fuse in the sub-run makesexec/2raise exactly like a non-zero process exit does natively — which is howlog:satisfiable falseworks.?_Nvariable numbering from a process-wide counter). The bridge caches sub-runs keyed on argument structure + file contents with variable numbering canonicalised by order of first appearance, so the second invocation is free — native EYE pays two process spawns here, the WASM build now pays one module boot (~85ms).executeBasicEyeQuery(i.e.n3reasoner/linguareasoner) now always drives the reasoner through the async yield loop with the bridge installed, and the CLI wraps its stdinlog:askcallback in the bridge too. A user-suppliedcbkeeps receiving all non-exec yields unchanged.Compatibility & perf
runQuery's synchronous cb-less path is kept (and now has an explicit test); only the internal invocation path ofexecuteBasicEyeQuerymoved fromqueryOnceto theqaQueryloop.queryOnce4.4ms vs async loop 3.3ms — indistinguishable within scheduler noise. Node memory leak tests (test:memory:node,test:memory:node:error) pass unchanged.log:askquestions without acboption now reject withThe reasoner yielded a question [...] but no cb option was provided to answer it; onmainthey rejected too, with** ERROR ** eam ** error(permission_error(run,goal,await(...))).pre-commit-style caveat remains for synchronous direct users ofqueryOnce: scoped builtins can only be bridged in the async path, soqueryOnce(module, 'main', args)on data with scoped builtins keeps failing (as today, but now with theexec_errorof an unanswerable yield rather than theshell/2error).Tests
New cases in
universalTests(run in node and jsdom, all green; full local run: 115 passed, 2 pre-existing skips; coverage thresholds met,lib/bridge.tsat 100%):(:x) a :Result.log:conclusionwithlog:includesover the closurelog:satisfiableincl. an unsatisfiable scope (sub-run inference fuse →false)e:execrejection with the descriptive error (plus malformed exec command lines)runQuerypathcbreject with the descriptive messageFollow-up / upstream ask
exec/2in the same emscripten conditional asuserInput/2, e.g.exec(A, 0) :- await(exec(A), _)— ideally yielding a structured term (args + output path) rather than a shell string? That would make the host contract explicit for any WASM embedder and let eye-js drop the build-time append (the host side here already anticipates it). Filed from the analysis in the #873 investigation.eye.plever changes theexec(A, B) :-helper shape,withWasmExecBridgeappends nothing and the scoped-builtin tests fail loudly at the next pvm regeneration, which is the intended tripwire.🤖 Generated with Claude Code