Debug button to show Datalog AST #16

justjake · 2022-05-01T17:51:11Z

Hello 👋🏻

what's going on in here?

I'm interested in compiling Percival's Datalog language to SQLite (sql.js) queries. But as a newcomer to both relational calculus and Rust, it's quite difficult for me to "think" down at the Rust level. To make the challenge more approachable, I want to expose the Percival AST into the notebook environment, which makes it much easier to explore and substantially tightens the experimentation loop.

what?

To that end, this PR makes the following changes:

Rename arrange codegen.rs -> codegen_js.rs to allow for multiple code generators. Introduce codegen_json, which is about as basic as it gets - it just serializes the AST. In the future, we could add a codegen_sql or whatever else here.
Hook up a --emit js | json flag for percival-cli. This allows easy piping of the AST to ~wherever it's needed during development
Add a build step to produce typescript definitions for the AST node JSON structure. This isn't strictly necessary right now, but could enable customized type-safe rendering of the AST.
Add result.ast() method to percival-wasm. There's some Makefile shenanigans (😱) to patch up the wasm-pack typedefs with the typesafe defs generated in (3). Happy to hear feedback about this
Add new Svelte component to render the AST JSON if ?debug is present in the URL

future plans?

I hope the changes in this PR are useful enough to stand on their own. If I have enough time, this is how I want to approach the SQL lowering challenge:

Add AST debug output (✅)
Allow code and plot cells to access AST data directly. This could enable us to build new Percival compilers... in Percival!?
Expose Percival's persistence (there's a file format?) and sharing options (Gists?) in the UI, so it's easier for everyone to do bigger things and save their work.
Experiment with SQL lowering use tooling in (1) and (2). Study the approach of EvgSkv/logica.
Once rules and algorithms are understood, implement it in Rust.

I have some other ideas for how to extend Percival's reach, inspired by twitter.com/tomlarkworthy but I'll save those for a stand-alone Github issue.

code quality?

I've never written any Rust or Svelte before, so let me know if something's off here!
I didn't measure bundle size changes to the wasm-pack output. Is that something I should worry about more?

infogulch · 2022-05-03T17:51:50Z

This seems interesting, I like my tools to be introspectable. It might be nice to have a 'view debug output' button somewhere on each cell to enable it without having to reload the page and lose edits. But first things first...

I tried to run locally, but make crates/percival-wasm/pkg failed on "Patch WASM types to use AST types". Cli transcript:

[~/percival][  debug][last: 0m 0s] make node_modules
npm install

> postinstall
> patch-package

patch-package 6.4.7
Applying patches...
[email protected] ✔

added 1 package, removed 6 packages, and audited 535 packages in 2s

131 packages are looking for funding
  run `npm fund` for details

3 high severity vulnerabilities

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

[~/percival][  debug][last: 0m 2s] make crates/percival-wasm/pkg
#       Build WASM
wasm-pack build --target web crates/percival-wasm
[INFO]: Checking for the Wasm target...
[INFO]: Compiling to Wasm...
   Compiling syn v1.0.85
   Compiling serde_derive v1.0.136
   Compiling serde v1.0.136
   Compiling serde_json v1.0.80
   Compiling ryu v1.0.9
   Compiling itoa v1.0.1
   Compiling termcolor v1.1.2
   Compiling Inflector v0.11.4
   Compiling wasm-bindgen v0.2.78
   Compiling wasm-bindgen-backend v0.2.78
   Compiling thiserror-impl v1.0.30
   Compiling ts-rs-macros v6.1.2
   Compiling wasm-bindgen-macro-support v0.2.78
   Compiling wasm-bindgen-macro v0.2.78
   Compiling thiserror v1.0.30
   Compiling ts-rs v6.1.2
   Compiling percival v0.1.0 (/home/joe/percival/crates/percival)
   Compiling console_error_panic_hook v0.1.7
   Compiling percival-wasm v0.1.0 (/home/joe/percival/crates/percival-wasm)
    Finished release [optimized] target(s) in 21.13s
[WARN]: :-) origin crate has no README
[INFO]: Installing wasm-bindgen...
[INFO]: Optimizing wasm binaries with `wasm-opt`...
[INFO]: Optional fields missing from Cargo.toml: 'description', 'repository', and 'license'. These are not necessary, but recommended
[INFO]: :-) Done in 28.56s
[INFO]: :-) Your wasm pkg is ready to publish at crates/percival-wasm/pkg.
# Build AST types
cargo test ast::export_bindings
   Compiling syn v1.0.85
   Compiling serde_derive v1.0.136
   Compiling serde v1.0.136
   Compiling serde_json v1.0.80
   Compiling ryu v1.0.9
   Compiling itoa v1.0.1
   Compiling wasm-bindgen v0.2.78
   Compiling Inflector v0.11.4
   Compiling scoped-tls v1.0.0
   Compiling maplit v1.0.2
   Compiling wasm-bindgen-backend v0.2.78
   Compiling proc-macro-error v1.0.4
   Compiling wasm-bindgen-macro-support v0.2.78
   Compiling thiserror-impl v1.0.30
   Compiling ts-rs-macros v6.1.2
   Compiling clap_derive v3.0.6
   Compiling wasm-bindgen-macro v0.2.78
   Compiling thiserror v1.0.30
   Compiling ts-rs v6.1.2
   Compiling clap v3.0.7
   Compiling percival v0.1.0 (/home/joe/percival/crates/percival)
   Compiling js-sys v0.3.55
   Compiling console_error_panic_hook v0.1.7
   Compiling wasm-bindgen-futures v0.4.28
   Compiling wasm-bindgen-test v0.3.28
   Compiling percival-wasm v0.1.0 (/home/joe/percival/crates/percival-wasm)
   Compiling percival-cli v0.1.0 (/home/joe/percival/crates/percival-cli)
    Finished test [unoptimized + debuginfo] target(s) in 22.25s
     Running unittests (target/debug/deps/percival-11350be32a61e7c9)

running 8 tests
test ast::export_bindings_aggregate ... ok
test ast::export_bindings_clause ... ok
test ast::export_bindings_fact ... ok
test ast::export_bindings_import ... ok
test ast::export_bindings_literal ... ok
test ast::export_bindings_rule ... ok
test ast::export_bindings_value ... ok
test ast::export_bindings_program ... ok

test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/parse.rs (target/debug/deps/parse-3dedbc611c1bf0d9)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 15 filtered out; finished in 0.00s

     Running unittests (target/debug/deps/percival_cli-135f0e1b8deb88c5)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests (target/debug/deps/percival_wasm-c85d9ec7031ec898)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/web.rs (target/debug/deps/web-3a45c2f311af40a1)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

mkdir -p crates/percival-wasm/pkg/ast
for ts in crates/percival/bindings/* ; do \
        cp $ts crates/percival-wasm/pkg/ast/"$(basename "${ts%.ts}.d.ts")" ; \
done
# Patch WASM types to use AST types
sed -i '' -e 's~ast(): any~ast(): import("./ast/Program").Program | undefined~' crates/percival-wasm/pkg/percival_wasm.d.ts
sed: can't read : No such file or directory
make: *** [Makefile:20: crates/percival-wasm/pkg] Error 2
[~/percival][  debug][last: 1m 34s]

justjake · 2022-05-03T18:19:38Z

sed issue is probably a BSD vs GNU thing

infogulch · 2022-05-03T18:27:29Z

That makes sense. How necessary is using the generated AST bindings to the functionality of the viewer? Personally, from a project simplicity perspective I would prefer to delay adding complex Makefiles (or any makefiles) as long as possible, and if not for this no changes to the project build would be required at all.

justjake · 2022-05-03T22:36:12Z

I can move those commands into package.json scripts if you find that more palatable.

infogulch · 2022-05-04T19:37:22Z

I was able to get it working (with the old build system) and it looks pretty neat:

It seems reasonable to merge, but I have some concerns. ekzhang may have a different view.

adding complexity to the build system for a mild benefit
- It looks like the purpose of Makefile lines 16-22 is to add types for the debug ASTView component. Is this a fair characterization? I'm not sure if its worth changing the root entry point of the build system (the most fragile and difficult to maintain part of most projects) to support type checking a single line in a debug feature.
- sed is not compatible across platforms
a few minor cleanup tasks
- Vestiges of old code (ast property of FullView component)
- Rust build fails due to a warning, see: https://github.com/ekzhang/percival/runs/6278144614?check_suite_focus=true
- Frontend build fails due to formatting issue, see: https://github.com/ekzhang/percival/runs/6278144391?check_suite_focus=true
I'd prefer a button to show the ast instead of a url param

I can make these changes on your branch if you prefer.

justjake · 2022-05-04T21:46:08Z

One of my eventual goals (per #18) is to publish the compiler to NPM; for that it would be good to provide typings for the AST.

justjake · 2022-05-04T21:47:04Z

Very happy to have assistance placing a debug button somewhere and cleaning up the code formatting issues. Probably can't get to any fixes myself until next week.

ekzhang

Interesting! I like the idea of having other kinds of code generation. I also learned something new today about generating TypeScript stubs from Rust types, that's interesting.

I think showing the AST is a cool idea but am not personally too keen on the patching and type generation magic. If the goal is to just be able to print show the Datalog AST in JavaScript code, I think it would make sense to add an ast() method to the wasm component, but I'm less certain about the other parts:

"Codegen JSON" seems like not really a necessary codegen module? It's just printing out the AST.
Nothing prevents codegen -> codegen_js from happening in the future when it's necessary if there's ever a future codegen.
The type generation from Rust types, while nice, I don't think is immediately useful to this PR and is a bit complex.

I also think the AST view could just be a setting in the actual interface, not necessarily requiring you to set ?debug in the URL.

Makefile

…utton; remove Makefile; document how to generate TS types in ast.rs

infogulch · 2022-05-06T18:42:36Z

I made a couple changes at @justjake's request:

Cleaned up formatting issues in ts & rust files
Changed ASTView to show based on a button
- This button behavior is a little janky, but it's a start
Removed references to ast type; use any instead (for now)
- Reverted README.md changes; removed Makefile
- Documented how to generate types in ast.rs based on the Makefile (kept for reference, this should probably be moved elsewhere)

I made a bit of a mess of the commits, but after getting my act together I squashed my changes down to just one.

This does not address @ekzhang's design question wrt codegen in rust.

justjake · 2022-05-06T21:11:06Z

src/lib/runtime.ts

@@ -13,7 +12,7 @@ type CompilerResultOk = {
  evaluate: (deps: Record<string, object[]>) => EvalPromise;
  deps: string[];
  results: string[];
-  ast: Program | undefined;
+  ast: any;


This is probably better as object | undefined

That makes sense.

infogulch · 2022-05-07T05:20:19Z

This might be better as a separate PR so we can focus on the functionality here, but what do we think about orienting the buttons vertically, and only showing delete and debug buttons when input is shown?

This prevents the addition of the bug icon from shifting the other buttons around and accommodates more buttons in the future. I like how the chevron is in the corner of the sidebar button.

But it's not perfect, if you have a section with no content then the buttons look weird:

justjake · 2022-05-07T21:05:09Z

Agree that the changes to the codegen files are unnecessary here. I'm happy to undo those changes and remove the type generation feature until it's being used.

I added the Makefile to encode the build process to produce the percival-wasm NPM module with correct AST types. That needs some build logic that was too long for direct inclusion in pacakge.json. I know Makefile is a polarizing tool. I prefer a Makefile to complex scripts inside pacakge.json, but the same tasks can be accomplished with either style.

Why include types in percival-wasm? I would like to experiment with codegen steps in Typescript, which is my current preferred programming language. Having types makes doing so much more pleasant. Again these changes can wait until needed.

justjake · 2022-05-07T21:38:55Z

Alright, I think this is ready for another look from you @ekzhang.

src/components/cell/CellOutput.svelte

infogulch · 2022-05-06T21:13:55Z

src/lib/runtime.ts

@@ -13,7 +12,7 @@ type CompilerResultOk = {
  evaluate: (deps: Record<string, object[]>) => EvalPromise;
  deps: string[];
  results: string[];
-  ast: Program | undefined;
+  ast: any;


That makes sense.

justjake added 5 commits May 1, 2022 07:04

new codegen type: json (emits the AST)

8fbc093

Add AST emit to percival-wasm

0140a7b

README: use make

31bbc36

render AST when ?ast in URL

28d7fea

show ast when ?debug

e9ea4fa

infogulch requested review from ekzhang and removed request for ekzhang May 3, 2022 17:18

ekzhang reviewed May 6, 2022

View reviewed changes

Makefile Outdated Show resolved Hide resolved

vercel bot deployed to Preview May 6, 2022 05:55 View deployment

infogulch force-pushed the main branch from b44e9fd to 6fe8aa4 Compare May 6, 2022 18:41

Prettier; move display logic into ASTView; display ASTView based on b…

6fe8aa4

…utton; remove Makefile; document how to generate TS types in ast.rs

justjake commented May 6, 2022

View reviewed changes

justjake mentioned this pull request May 7, 2022

Plot cell: allow async, multiple inputs, one output #19

Open

justjake added 3 commits May 7, 2022 14:14

slightly tighter type for ast

55ff95a

undo codegen_js stuff

cd78df5

ignore TS bindings files

b6c8de2

justjake changed the title ~~Show datalog AST when ?debug is in the URL~~ Debug button to show Datalog AST May 7, 2022

Revert README.md change

5632e37

infogulch approved these changes May 7, 2022

View reviewed changes

infogulch mentioned this pull request May 7, 2022

Sidebar UX #20

Open

vercel bot deployed to Preview May 12, 2022 07:12 View deployment

infogulch requested a review from ekzhang June 17, 2022 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Debug button to show Datalog AST #16

Debug button to show Datalog AST #16

justjake commented May 1, 2022 •

edited

Loading

infogulch commented May 3, 2022

justjake commented May 3, 2022

infogulch commented May 3, 2022 •

edited

Loading

justjake commented May 3, 2022

infogulch commented May 4, 2022 •

edited

Loading

justjake commented May 4, 2022

justjake commented May 4, 2022

ekzhang left a comment

infogulch commented May 6, 2022

justjake May 6, 2022

infogulch May 6, 2022

infogulch commented May 7, 2022 •

edited

Loading

justjake commented May 7, 2022

justjake commented May 7, 2022

infogulch May 6, 2022

Debug button to show Datalog AST #16

Are you sure you want to change the base?

Debug button to show Datalog AST #16

Conversation

justjake commented May 1, 2022 • edited Loading

what's going on in here?

what?

future plans?

code quality?

infogulch commented May 3, 2022

justjake commented May 3, 2022

infogulch commented May 3, 2022 • edited Loading

justjake commented May 3, 2022

infogulch commented May 4, 2022 • edited Loading

justjake commented May 4, 2022

justjake commented May 4, 2022

ekzhang left a comment

Choose a reason for hiding this comment

infogulch commented May 6, 2022

justjake May 6, 2022

Choose a reason for hiding this comment

infogulch May 6, 2022

Choose a reason for hiding this comment

infogulch commented May 7, 2022 • edited Loading

justjake commented May 7, 2022

justjake commented May 7, 2022

infogulch May 6, 2022

Choose a reason for hiding this comment

justjake commented May 1, 2022 •

edited

Loading

infogulch commented May 3, 2022 •

edited

Loading

infogulch commented May 4, 2022 •

edited

Loading

infogulch commented May 7, 2022 •

edited

Loading