ci: run lsp-bench 0.3.3 so the file-ops lifecycle actually executes - #232
Draft
rifuki wants to merge 1 commit into
Draft
ci: run lsp-bench 0.3.3 so the file-ops lifecycle actually executes#232rifuki wants to merge 1 commit into
rifuki wants to merge 1 commit into
Conversation
Commit 556208f renamed the benchmark keys to their lsp-bench 0.3.x spellings (renameSteps/createSteps/deleteSteps -> renameFiles/createFiles/deleteFiles), but preflight still installs 0.2.7, which does not know those keys. The effect is quieter than a failure. 0.2.7 declares the field as `#[serde(default, rename = "renameSteps")]` with no deny_unknown_fields, so the new blocks are silently dropped and it falls back to a single one-shot operation against the top-level `file:` using a generated placeholder name. The step has been exercising *a* rename, just never the one the config describes — in particular the A.sol -> AA.sol -> A.sol round-trip, whose whole purpose is to catch stale caches and corrupted imports on the second rename, has not run since May. That is the failure class CHANGELOG v0.1.34 documents. Verified locally against this checkout with foundry on PATH: 0.2.7 [1/3] workspace/willRenameFiles (one-shot, placeholder name) 0.3.3 [1/3] workspace/willRenameFiles rename 2 rename step(s) (full lifecycle) -> 4 file(s) with edits -> 4 file(s) with edits The bump also un-breaks more than the file-ops step: all seven code-action configs fail outright under 0.2.7 with "unknown benchmark 'textDocument/codeAction'", and waitForProgressToken starts working. Adds a liveness gate, because --verify is not one. Pointed at a stub server that exits immediately, both ci-verify.yaml and ci-file-ops-verify.yaml print "verify 0/0 expectations passed" and exit 0 — lsp-bench treats "nothing answered" as "nothing to check". A binary that builds but dies on startup would pass preflight today. The workflow now runs `--version` first, and both lsp-bench steps get the same `timeout` wrapper the cargo test step above them already has (version detection happens before any per-request timeout, so a non-responsive server hangs the step indefinitely). ci-file-ops-verify.yaml gains a documentSymbol expectation so --verify has something it can actually evaluate here: the file-op responses are WorkspaceEdits, and lsp-bench ignores per-step `expect` blocks while a method-level `minCount` counts array items ("expected >= 4 but got 0"). It runs after the round-trip restores A.sol, so it also catches a lifecycle that corrupted the file it was meant to put back. Known regression from the bump, disclosed rather than fixed here: 0.3.3 records the first inbound notification as the `textDocument/diagnostic` response — usually `{"message": "lsp server initialized."}` — where 0.2.7 recorded the publishDiagnostics payload. Timings stay valid, the captured response does not, so benchmarks/poolmanager-t-diag.yaml gains a warning not to publish a diagnostics report from it until lsp-bench waits for publishDiagnostics. Worth filing upstream. Also fixes benchmarks/servers.yaml: the `latest` entry pointed at an absolute path under another developer's home directory, so every documented comparison run resolved to a nonexistent binary for anyone else. It now uses the same repo-relative path the v0.1.25 entry above it already uses — and that v0.1.25 entry is flagged stale, since it points at the current build rather than at v0.1.25 and would make the A/B in pool.yaml compare a build against itself.
rifuki
marked this pull request as draft
August 25, 2026 10:01
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
1 participant
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.
556208frenamed the benchmark keys to lsp-bench 0.3.x spellings (renameSteps→renameFilesand friends), but preflight still installs 0.2.7, which doesn't know them.It fails quietly rather than loudly. 0.2.7 declares the field as
#[serde(default, rename = "renameSteps")]with nodeny_unknown_fields, so the blocks are dropped and it falls back to one shot against the top-levelfile:with a placeholder name. The step has been running a rename, just not the configured one — so theA.sol → AA.sol → A.solround-trip, which exists to catch stale caches on the second rename, hasn't run since May.Ran both locally against this checkout:
While testing this I found something worth flagging separately:
--verifydoesn't catch a dead server. Pointed at a stub that exits immediately, both CI configs printverify 0/0 expectations passedand exit 0 — lsp-bench treats "nothing answered" as "nothing to check". A binary that builds but dies on startup passes preflight today, before and after this bump. So the workflow now runs--versionfirst, and both lsp-bench steps get thetimeoutwrapper thecargo teststep above them already has.ci-file-ops-verify.yamlalso gets adocumentSymbolexpectation, since that's something--verifycan actually evaluate here — the file-op responses areWorkspaceEdits, and lsp-bench ignores per-stepexpectblocks while a method-levelminCountcounts array items.One regression from the bump, which I've disclosed rather than fixed: 0.3.3 records the first inbound notification as the
textDocument/diagnosticresponse — usually{"message": "lsp server initialized."}— where 0.2.7 recorded the publishDiagnostics payload. Timings are still valid, the captured response isn't, sopoolmanager-t-diag.yamlgets a warning comment. Probably worth an upstream issue.Also fixes
servers.yaml: thelatestentry pointed at an absolute path under someone's home directory, so comparison runs resolved to a nonexistent binary for everyone else. Happy to split that out if you'd rather keep the CI pin isolated.Both CI configs exit 0 under 0.3.3 and
example/is left untouched.