Skip to content

Add information about clauses to implication cards - #424

Merged
gares merged 5 commits into
LPCIC:masterfrom
VojtechStep:impl-trace
Jul 17, 2026
Merged

Add information about clauses to implication cards#424
gares merged 5 commits into
LPCIC:masterfrom
VojtechStep:impl-trace

Conversation

@VojtechStep

Copy link
Copy Markdown
Contributor

It's useful to see in the stacktrace what rules were added to the context. It could probably be done with better UI, but this is the least invasive version.

Example stack trace
image

@gares

gares commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Nice!

Two comments:

  • I don't get why we get "implication" printed twice (but I don't recall much of the trace elaborator)
  • Maybe you should print the extra info inside add_clauses, since the rule you add may be compiled into multiple rules, eg (p, q) => r should give two entries, p and q and not one entry (p,q) IMO.

@VojtechStep

Copy link
Copy Markdown
Contributor Author

By implication being printed twice you mean in the stack trace in VSCode? That's because I manually expanded the implication steps in the UI.

I'll mark this as draft and cook a bit more

@VojtechStep
VojtechStep marked this pull request as draft July 11, 2026 10:26
@VojtechStep
VojtechStep force-pushed the impl-trace branch 2 times, most recently from a93cb00 to e7e9ce3 Compare July 15, 2026 16:46
@VojtechStep

Copy link
Copy Markdown
Contributor Author

I added support for pushing multiple stack frames per step, updated the tests, and added new ones.

This is what the stack look like for the test
image

and here it is with the implication steps expanded
image

It'd be better if the "Logic - implication" wasn't there, or ideally if there was a separate window for local assumptions (and, tangentially, maybe another one for showing assigned unification variables), but I think this is as far as I can go without touching the extension code.

@VojtechStep
VojtechStep marked this pull request as ready for review July 15, 2026 17:00
It's useful to see in the stacktrace what rules were added to the
context. It could probably be done with better UI, but this is the
least invasive version.
@VojtechStep

Copy link
Copy Markdown
Contributor Author

The last push should fix the 4.14 errors (how is List.is_empty that recent?!), but I don't think I can do anything about the Windows situation (though I see that it also fails on master)

@gares

gares commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

With the commit I pushed and this LPCIC/elpi-lang#35 I get
Screenshot From 2026-07-16 14-50-01
when there are longer rules, they are elided as usual

@gares

gares commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

oops I forgot to promote and I have to go now. Please give it a try and if it works for you I'll merge

@VojtechStep

Copy link
Copy Markdown
Contributor Author

Funny that you removed Shiki from the extension, I just fixed the integration this morning, and I've been looking into fixing the terrible performance (which I believe is fixable).

I think the better approach to backwards compatibility would be to have a meta packet as the first object of the trace, which specifies the data version (and if it's missing treat it as v1).

I can try implementing that if you'd like.

@gares

gares commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

About versioning the trace format, yes, that would be a more principled approach, but one would still need to pass the version around to print things differently, or version&copy the code.

I thought about that and decided it was not worth it for this little change. But if you have many changes planned, it may make perfect sense

@gares
gares merged commit c56c23f into LPCIC:master Jul 17, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants