Skip to content
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

Significant proof checking refactoring #46

Merged
merged 33 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
2361b69
Implement base use of feature to check proofs. Needs fixes
K-dizzled Jul 29, 2024
9e9b6e1
Test and integrte command arg of getGoals to check proofs
K-dizzled Oct 9, 2024
415f71d
Merge main into core-lsp-rework
K-dizzled Oct 16, 2024
d5c66ac
Upgrade coq-lsp in actions to v0.2.2 for arg in getGoals
K-dizzled Oct 16, 2024
09c957e
Change type of CoqLspClient to Interface everywhere possible
K-dizzled Oct 16, 2024
e32b7f9
Clean CoqLspClientInterface
K-dizzled Oct 16, 2024
9125619
Refactor `CoqProofChecker`
K-dizzled Oct 17, 2024
ad795e7
Clean-up
K-dizzled Oct 17, 2024
872987b
Refactor creating in plugin mode: now once in
K-dizzled Oct 18, 2024
e97e1ec
Create decorator that logs execution time of the method
K-dizzled Oct 20, 2024
5baf1f8
Make computation of `initialGoal` optional
K-dizzled Oct 20, 2024
9319114
Create permanent status bar button
K-dizzled Oct 20, 2024
b2c18bf
Add refactor TODO to
K-dizzled Oct 20, 2024
3f44690
Sketch aborting the completion generation through `AbortSignal`
K-dizzled Oct 25, 2024
9689e85
Refactor `SourceFileEnvironment`: remove dirPath and rename
K-dizzled Oct 25, 2024
095b99b
Refactor `SourceFileEnvironment`: remove `fileLines`
K-dizzled Oct 25, 2024
df91211
Refactor `checkGeneratedProofs`
K-dizzled Oct 25, 2024
632c80a
Minor changes after sketching completion abort in extension
K-dizzled Oct 26, 2024
7530442
Merge branch 'core-lsp-rework' into extension-completion-abort
K-dizzled Oct 26, 2024
e56e10e
Merge pull request #45 from JetBrains-Research/extension-completion-a…
K-dizzled Oct 26, 2024
d8f591f
Fix merge of `abort-completion` into `core-lsp-refactor`
K-dizzled Oct 26, 2024
0ec7316
Add TODOs on `LspCoreRefactor`
K-dizzled Oct 26, 2024
cdd87b3
Create `SessionExtensionState`
K-dizzled Oct 27, 2024
34bdfa2
Refactor `CoqLspClient` to `CoqLspClientImpl`
K-dizzled Oct 27, 2024
bab8a26
Finalize aborting `coq-lsp` gracefully
K-dizzled Oct 27, 2024
e122675
Prepare for v2.3.1 release
K-dizzled Oct 27, 2024
33d1197
Fix cosmetics from review in PR#46
K-dizzled Nov 6, 2024
d1d4952
Fix more issues from review in PR#46
K-dizzled Nov 6, 2024
09a3420
Code style fixes and core in rankers from review in PR#46
K-dizzled Nov 6, 2024
c58c89f
Refactoring `healthStatusIndicator` according to review in PR#46
K-dizzled Nov 6, 2024
a8ac5b8
Renamings according to review in PR#46
K-dizzled Nov 6, 2024
0b36cc3
More accurate `completionAbortError` handling
K-dizzled Nov 6, 2024
f5122cd
Refactor `coqLsp` tests according to review of PR#46
K-dizzled Nov 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ on:

env:
coqlsp-path: "coq-lsp"
coqlsp-version: "0.1.9+8.19"
coqlsp-version: "0.2.2+8.19"
artifact-name: ubuntu-latest-build

jobs:
Expand Down Expand Up @@ -60,7 +60,7 @@ jobs:
env:
OPAMYES: true
run: |
opam install coq-lsp.0.1.9+8.19
opam install coq-lsp.0.2.2+8.19
eval $(opam env)

- name: Install Node.js
Expand Down
3 changes: 2 additions & 1 deletion .prettierrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
"printWidth": 80,
"importOrder": ["^[./]*llm/(.*)$", "^[./]*coqLsp/(.*)$", "^[./]*core/(.*)$", "^../", "^./"],
"importOrderSeparation": true,
"importOrderSortSpecifiers": true
"importOrderSortSpecifiers": true,
"importOrderParserPlugins": ["typescript", "@trivago/prettier-plugin-sort-imports", "decorators-legacy"]
}
19 changes: 11 additions & 8 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
// Place your settings in this file to overwrite default and user settings.
{
"files.exclude": {
"**/*.vo": true,
"**/*.vok": true,
"**/*.vos": true,
"**/*.aux": true,
"**/*.glob": true,
"**/.git": true,
"**/.svn": true,
"**/.hg": true,
"**/CVS": true,
"**/.DS_Store": true,
"**/Thumbs.db": true,
"**/*.vo": true,
"**/*.vok": true,
"**/*.vos": true,
"**/*.aux": true,
"**/*.glob": true,
"out": false,
"**/*_cp_aux.v": true
"out": false
},
"search.exclude": {
"out": true // set this to false to include "out" folder in search results
},
// Turn off tsc task auto detection since we have the necessary tasks as npm scripts
"typescript.tsc.autoDetect": "off"
"typescript.tsc.autoDetect": "off",
"coq-lsp.check_only_on_request": false,
"eslint.options": {
"experimentalDecorators": true
}
}
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# Changelog

## 2.3.1

The version increment is minor, yet the changes are significant. The main focus of this release was to improve interaction with the `coq-lsp` server. We now completely got rid of temporary files in the `extension` part of `CoqPilot`. This was done thanks to [ejgallego](https://github.com/ejgallego) and his improvements in `coq-lsp`. Now checking of proofs is done via the `proof/goals` request via `command` parameter, which is much more reliable and faster. Now a single `lsp` server is created for the plugin until being explicitly closed, and it tracks changes in the file. If you are using `coq-lsp` plugin itself, it will help you to always keep track of whether file is checked or not. When `CoqPilot` is ran on a completely checked file, it will not bring any significant overhead apart from requests to the Completion Services.

### Public changes
- Added a toggle button to enable/disable the extension
- Using the toggle switch, user can now abort the proof generation process
- Significant performance improvements in proof checking that are especially easy to see for large files

### Internal changes
- Get rid of temporary files in the `extension` part of `CoqPilot`
- Refactor `CoqLspClient`
- Make computation of `initialGoal` for premises optional to avoid unnecessary requests to `coq-lsp`

## 2.3.0

**A major upgrade of the benchmarking system**: At the moment, only a little **new** functionality is provided; moreover, the ability to run benchmarks on Tactician/CoqHammer is temporarily unavailable. However, it will soon be restored and improved. Excessive work has been done to make the benchmarking system more flexible, secure, robust, self-contained, and easy to use. Experiments via our benchmarking framework have been made more accessible than ever. The configurability and reliability of the pipeline have been improved drastically. In a nutshell, the main features of the improved benchmarking system include:
Expand Down
8 changes: 3 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
# CoqPilot

![Version](https://img.shields.io/badge/version-v2.3.0-blue?style=flat-square)
# CoqPilot ![Version](https://img.shields.io/badge/version-v2.3.1-blue?style=flat-square)

*Authors:* Andrei Kozyrev, Gleb Solovev, Nikita Khramov, and Anton Podkopaev, [Programming Languages and Tools Lab](https://lp.jetbrains.com/research/plt_lab/) at JetBrains Research.

Expand Down Expand Up @@ -30,7 +28,7 @@

## Requirements

* `coq-lsp` version `0.1.9+8.19` is currently required to run the extension.
* `coq-lsp` version `0.2.2+8.19` is currently required to run the extension.

## Brief technical overview

Expand Down Expand Up @@ -68,7 +66,7 @@ As soon as at least one valid proof is found, it is substituted in the editor an

To run the extension, you must install a `coq-lsp` server. Depending on the system used in your project, you should install it using `opam` or `nix`. A well-configured `nix` project should have the `coq-lsp` server installed as a dependency. To install `coq-lsp` using `opam`, you can use the following commands:
```bash
opam pin add coq-lsp 0.1.9+8.19
opam pin add coq-lsp 0.2.2+8.19
opam install coq-lsp
```
For more information on how to install `coq-lsp` please refer to [coq-lsp](https://github.com/ejgallego/coq-lsp).
Expand Down
58 changes: 26 additions & 32 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"url": "https://github.com/JetBrains-Research/coqpilot"
},
"publisher": "JetBrains-Research",
"version": "2.3.0",
"version": "2.3.1",
"engines": {
"vscode": "^1.82.0"
},
Expand Down Expand Up @@ -392,6 +392,7 @@
"pretest": "npm run compile && npm run lint",
"test": "npm run test-only",
"clean": "rm -rf out",
"rebuild": "npm run clean && npm run compile && npm run format",
"rebuild-test-resources": "cd ./src/test/resources/coqProj && make clean && make",
"preclean-test": "npm run clean && npm run rebuild-test-resources && npm run compile && npm run lint",
"clean-test": "npm run test-only",
Expand Down Expand Up @@ -441,7 +442,7 @@
"event-source-polyfill": "^1.0.31",
"i": "^0.3.7",
"mocha-param": "^2.0.1",
"node-ipc": "^11.1.0",
"node-ipc": "^12.0.0",
"npm": "^10.4.0",
"openai": "^4.6.0",
"path": "^0.12.7",
Expand All @@ -450,6 +451,7 @@
"tiktoken": "^1.0.17",
"tmp": "^0.2.3",
"toml": "^3.0.0",
"ts-results": "^3.3.0",
"vscode-languageclient": "^9.0.1",
"yargs": "^17.7.2"
}
Expand Down
33 changes: 0 additions & 33 deletions set_gitignore.sh

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from "../../../../../../core/coqProofChecker";

import { stringifyAnyValue } from "../../../../../../utils/printers";
import { Uri } from "../../../../../../utils/uri";
import { BenchmarkingLogger } from "../../../../logging/benchmarkingLogger";
import { LogsIPCSender } from "../../../../utils/subprocessUtils/ipc/onParentProcessCallExecutor/logsIpcSender";
import { TimeMark } from "../../measureUtils";
Expand Down Expand Up @@ -37,12 +38,20 @@ export namespace CheckProofsImpl {

try {
const timeMark = new TimeMark();
const fileUri = Uri.fromPath(args.fileUri);

// TODO: [@Gleb Solovev] Pay Atteniton that it was previously done by the CoqProofChecker
await coqLspClient.openTextDocument(fileUri);

const proofCheckResults = await coqProofChecker.checkProofs(
args.sourceFileDirPath,
args.sourceFileContentPrefix,
args.prefixEndPosition,
fileUri,
args.documentVersion,
args.checkAtPosition,
args.preparedProofs
);

await coqLspClient.closeTextDocument(fileUri);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure we are on the same page: am I right that in the correct implementation this closeTextDocument should be called in the finally case, while checkProofs should be wrapped into try one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understood the question( Well, in general, I guess the answer to your question is yes. The promised returned by method checkProofs could be rejected, yet, other errors shall not be thrown.

closeDocument ideally should happen in some wrapper over the coqProofChecker. Btw, would be neat to do smth like:

withOpenAuxFile: 
    ...


const proofsValidationMillis = timeMark.measureElapsedMillis();
return buildSuccessResult(
proofCheckResults,
Expand Down
Loading