Skip to content

Fix LSP workspace discovery, graph invalidation, and document close - #153

Merged
itsfuad merged 3 commits into
mainfrom
fix/workspace-empty-directory-discovery
Oct 4, 2026
Merged

itsfuad merged 3 commits into
mainfrom
fix/workspace-empty-directory-discovery

Conversation

@itsfuad

@itsfuad itsfuad commented Oct 4, 2026

Copy link
Copy Markdown
Member

Summary

  • What changed: Restore complete workspace discovery, correct graph invalidation and didClose decoding, reduce redundant rebuild work, and unify in-memory source terminology as sourceOverride.
  • Why this change is needed: Sources added inside previously empty directories could remain undiscovered. Same-count replacements could retain stale graph/components after read failure. Standard editor close notifications were ignored.
  • Owning phase/package/component: internal/lsp, with source-input terminology updates in driver, pipeline, prelude, and architecture docs.
  • Scope boundaries / non-goals: Module-level workspace correctness and existing rebuild optimizations. No language semantics, persistent cache, function-level reuse, or broad compiler architecture changes.

Production ownership

  • Current non-test production consumer(s) of each new production symbol: Run consumes DidCloseTextDocumentParams; workspace rebuild and compiler/query paths consume SourceOverrides; import-path derivation and resolution use rebuild-local contextForProject.
  • Execution path showing where the new behavior is reached in production: Editor notifications → Run → applyDocumentSnapshot → diagnostic publication/recompilation → workspaceIndex.rebuild.
  • Existing implementation reused, extended, replaced, or intentionally left unchanged: Reuse canonical project.DiscoverSourceFiles, manifest/project resolution, document mutation, diagnostic publication, import resolution, and graph/component construction.
  • Wrappers, aliases, duplicate paths, compatibility shims, or experimental scaffolding removed: Remove incomplete directory-stamp discovery shortcut, redundant rebuild membership map, and one-field workspaceProjectLookup. Rename source-input consumers directly without compatibility aliases.
  • Any new production symbol without a current production consumer: None.

Design and behavior

  • Previous behavior: Discovery could reuse an incomplete file list. Graph invalidation was initialized before full membership comparison. Close handling expected top-level uri. Warm-open benchmark supplied no open-buffer text.
  • New behavior: Every rebuild performs canonical discovery. Membership counts and keys determine graph invalidation before content reads. Close handling decodes textDocument.uri. Sorted discovery output is reused directly when no overrides exist; import contexts are constructed lazily and retained only within rebuild.
  • Key invariants preserved: Canonical paths, discovery exclusions, nearest-project/source-root filtering, project isolation, negative lookup caching, content/parse reuse, import resolution, diagnostic locking/generations, and nil-versus-empty source selection.
  • Intentional behavioral changes: Newly populated directories become visible; unreadable same-count replacements invalidate stale graph/components; standard close notifications discard buffer text/version and publish updated diagnostics.
  • Error / edge-case handling: Discovery errors propagate. Per-file read failures retain existing handling while membership still invalidates graph. Malformed close params and invalid URIs are ignored. Empty source overrides remain authoritative.
  • Why this implementation belongs in this layer rather than another phase/package: LSP owns document lifecycle, workspace membership, diagnostic selection, and incremental reuse. Canonical filesystem and compiler behavior remain with existing owners.

Validation

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache go test -count=1 -run '^(TestLSPDidCloseDiscardsSourceOverrideAndVersion|TestWorkspace.*|TestServerStateRechecksImporterWhenTargetMembershipChanges|TestRecompileUsesEmptySourceOverride|TestManifestLoadFailuresPublishOnSourceURI|TestMalformedNotificationURI.*|TestDocumentMutationWaitsForCheckedDiagnosticPublication|TestDiagnosticSnapshotCopiesComponentFiles)$' ./internal/lsp
PASS — compiler/internal/lsp 0.128s

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache go test -count=1 ./internal/project ./internal/pipeline ./internal/driver
PASS — project 0.018s; pipeline 0.778s; driver 0.009s

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache go test -race -count=1 ./internal/lsp
PASS — compiler/internal/lsp 47.588s

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache go test -count=1 ./...
PASS — all packages; compiler/internal/lsp 38.612s

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache go vet ./...
Exit 0; no output

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache go run ./scripts/bundle.go
Exit 0; fresh bundle generated

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache PEEPER_BIN="$PWD/build/bin/peeper" go test -count=1 ./x_test
PASS — compiler/x_test 27.061s

env CCACHE_DISABLE=1 GOCACHE=/tmp/omnirush/peeper-go-cache go test -run '^$' -bench '^BenchmarkIncrementalWorkspace$/^small$/^(warm_no_change_open|function_body_edit|import_set_edit)$' -benchmem -benchtime=200ms -count=3 ./internal/lsp
PASS — 10.065s; body edit: 1 parsed/4 reused; import edit: 2 parsed/4 reused
Timing ranges overlap before/after; no general editor-latency gain claimed

git diff --check
Exit 0; no output
  • Targeted tests added or updated: Empty-directory add/rename/remove/re-add parity; unopened-file diagnostic publication; protocol-level disk-backed/unsaved close cleanup; same-count unreadable replacement; multi-project initial/remove/restore isolation.
  • Existing regression coverage exercised: Workspace imports/components, manifest refresh/failures, incremental parse/reuse, empty source overrides, diagnostic generation/publication locking, and bundled Peeper fixtures.
  • Manual verification, if applicable: Fresh bundled CLI smoke confirms standard close removes hover-visible buffer text and version. Flat params, invalid URI, and malformed document payloads preserve state. Formatting check produced no diff across all 13 changed Go files.
  • Checks not run: Native Windows/macOS execution; local validation ran on Linux/amd64.

Diff sanity

  • Unrelated production changes: None.
  • Duplicated logic introduced: None.
  • Dead or unreachable code introduced: None.
  • Temporary/debug code remaining: None.
  • Comments/docs that became stale because of this change: LSP and infrastructure architecture docs updated for discovery, invalidation, protocol close, and source-override terminology.

Risks and follow-up

  • Known risks or assumptions: Complete discovery costs more than the removed invalid shortcut. Direct-result reuse relies on canonical discovery’s sorted, deduplicated output. Pointer-valued lookup maps must retain checked-nil results.
  • Behavior intentionally changed: Workspace membership refresh and standard document-close handling, as described above.
  • Behavior intentionally not changed: Language semantics, compiler phase boundaries, per-file parse reuse, diagnostic ownership, and persistent caching.
  • Remaining gaps: Cross-platform execution awaits CI. Read-failure regression skips Windows if source-symlink creation is unavailable. rebuild remains dense; concrete sequencing and indirection issues were repaired without broader extraction.
  • Linked issues / follow-up work: Further manifest-probe and diagnostic-batch reuse remain separate, measurement-driven work.
  • Anything that should block merge: Required CI checks and approving review from a non-author human collaborator.

@itsfuad itsfuad added this to the Tooling and Package Management milestone Oct 4, 2026
@itsfuad
itsfuad merged commit cfcae13 into main Oct 4, 2026
17 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.

1 participant