Skip to content

Checker & hover bug fixes, results-first profiler UX, shipped profiling UI, and a truthful profiler docs rewrite#298

Merged
abdushakoor12 merged 13 commits into
mainfrom
nimble/vibrant-hopper-8364f3
Jul 8, 2026
Merged

Checker & hover bug fixes, results-first profiler UX, shipped profiling UI, and a truthful profiler docs rewrite#298
abdushakoor12 merged 13 commits into
mainfrom
nimble/vibrant-hopper-8364f3

Conversation

@abdushakoor12

@abdushakoor12 abdushakoor12 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

TLDR

Six user-reported bug fixes (four checker/CLI precision bugs, three hover gaps), a profiler UX overhaul that lands every profile on Basilisk's own results views with speedscope loading automatically, the profiling UI officially shipped (availability gate deleted), and a ground-truth rewrite of the profiler documentation — with coverage ratchets moved up to match.

References: issue #285, issue #283, issue #282, issue #279, issue #278, issue #287, issue #288, issue #289 .

What Was Added?

  • Hover on dot-access members (crates/basilisk-lsp/src/hover/members.rs): hovering a method inherited from an external base (e.g. pydantic.BaseModel.model_validate, issue pydantic BaseModel model_validate don't have hover #287) resolves the receiver through a cycle-guarded, BFS base-class walk into imported_symbols; builtin receivers get signatures from a new curated table of all 47 public str methods (crates/basilisk-stubs/src/builtin_members.rs, issue method don't have on hover #288); hovering a class name now appends its own-or-inherited __init__ signature (issue hover on class don't have init hint #289).
  • Method data through the stub pipelines: ExternalSymbol gains a methods: Vec<ExternalMethod> field, populated by workspace export extraction, .pyi parsing, and runtime-introspection stub generation (which now emits class bodies instead of class X: ...).
  • Loopback profile server (vscode-extension/src/profile-server.ts): serves exported profiles over http://127.0.0.1:<port>/<token>/<basename> so the speedscope.app deep link loads automatically instead of dumping the user on the empty Browse page. Loopback-only bind, unguessable 32-hex token per file, GET-only, no-store, 10-minute expiry, torn down on deactivate.
  • New profiler affordances: "Basilisk: Show Profile Results" palette command re-opens the last result; the CPU results panel gains "Open Trace in VS Code Viewer"; the memory dashboard gains "Open Heap Profile in VS Code Viewer" and "Open in Speedscope (external)" (speedscope imports V8 sampling heap profiles); snapshot toasts carry "View Results".
  • Shared base-class matcher (crates/basilisk-checker/src/rules/shared.rs::class_or_base_matches): one transitive, cycle-guarded walk now backs guards, missing-override, TypedDict-required, NamedTuple, protocol, enum, and dataclass-transform rules that each had a private copy.

What Was Changed or Deleted?

  • Checker/CLI precision fixes: names bound in try/except/else/finally are computed path-correctly, ending names_unbound false positives on try-assigned names (issue False positive "returns o but o may be unbound on some paths" #285); TypeVar(bound=Callable[..., Any]) parses instead of tripping generics_basic (issue False positive "has a bound= that is parameterized by a type variable" #283); an empty dict()/{} is assignable to a dict type alias (issue Incorrect assignability with dict alias #282); CLI snippet gutters align with the source row (issue Red squiggles are misaligned #279); base-walks stay cycle-safe when a class shadows its external base's name (issue Stack overflow on VS Code when opening a large project #278 discipline).
  • Profiler landing inversion: stopping a CPU profile opens the Basilisk Profiler panel (summary cards, flame graph, click-to-source tables) instead of the raw .cpuprofile table; taking a memory snapshot opens the Basilisk memory dashboard instead of the raw .heapprofile. The native V8 viewers remain one deliberate click away everywhere, and an explicit request that cannot open is announced and reveals the file — never a silent failure.
  • Profiling UI ships enabled: profiling-ui.ts (the test-only availability gate, left force-enabled with a "do not commit" comment) is deleted; all 24 basilisk.profilingEnabled manifest references removed, keeping only real session-state when clauses. The spec section now guards the residue risk: a when clause on a context key nobody sets silently hides UI from every shipped user.
  • hover.rs split into hover/{mod,members,tests}.rs to respect the 500-LOC file limit.
  • Coverage ratchets (up only): basilisk-cli 89→94, basilisk-stubs 91→92, vsix 91→92.

How Do The Automated Tests Prove It Works?

  • Checker fixes each carry a regression test written to fail before the fix: names_unbound_tests (try/except binding, issue False positive "returns o but o may be unbound on some paths" #285), generics_basic_tests (TypeVar with Callable bound, issue False positive "has a bound= that is parameterized by a type variable" #283), assignment_compatibility_tests (empty-dict → alias, issue Incorrect assignability with dict alias #282), and the CLI gutter-alignment assertion in basilisk-cli/src/output/mod.rs (issue Red squiggles are misaligned #279).
  • Hover fixes: test_hover_on_method_inherited_from_external_stub_base_shows_signature builds a real pydantic.pyi, resolves imports, and asserts the full BaseModel.model_validate(obj: object) -> BaseModel signature; test_hover_on_str_literal_method_shows_signature asserts str.join on a string literal; test_hover_on_class_shows_init_signature asserts the __init__ block; entries_to_pyi_emits_class_methods pins runtime-stub method emission.
  • Profiler UX: the real debug-launch CPU e2e asserts the results panel opens on stop with zero toast clicks and stays reachable via basilisk.profileShowResults after closing; profile-server.test.ts pins the served body, CORS header, no-store, 404-on-wrong-token, registration expiry, and post-dispose unreachability; webview unit tests pin every new button (and their absence when no trace exists); a manifest sweep test fails if basilisk.profilingEnabled ever reappears.
  • Whole-pipeline gates on the merged tree: conformance 100% pass / 0 false positives via the unmodified python/typing scorer; mutation testing 125 mutants — 118 caught, 0 missed, 100% kill rate (baseline unchanged); VS Code suite 510 passing at 93% coverage; Neovim 43%; all Rust crates at or above their (raised) thresholds; deslop duplication gate at 12.53% < 12.6%.

Spec / Doc Changes

  • docs/specs/LSP-PROFILING-SPEC.md: [PROFILE-UI-GATE] rewritten as shipped-enabled UI availability; [PROFILE-NATIVE]/[PROFILE-NATIVE-FALLBACK] describe the results-view-first routing; [PROFILE-VIEWER-DELIVERY] documents the loopback deep-link flow and its containment; [PROFILE-MEMORY-FINAL] matches the dashboard-first landing. docs/PROFILER-BRANCH-AUDIT.md records the conform-5 sign-off.
  • website/src/docs/profiler.md (+ zh mirror) rewritten against the implementation: removed fabricated content (pyproject profiler config, CPU "Compare Profiles", GIL detection, percentage leak scores, basilisk-profiles/ directory), corrected heat thresholds (20/10/5/1), diagnostic codes/severities (BSK-PROF-* Hints, BSK-MEM-* family), command names across VS Code/Zed/Neovim, and added the Python Processes panel flow, cooperative macOS sampler, memory autopilot, and the full 12-setting configuration table. quick-start.md, install-zed.md, and the docs index (en + zh) swept for the same stale claims.

Breaking Changes

  • None

collect_unconditional_assigns had no Stmt::Try arm, so a name assigned
in both the try body and every except handler was never counted as
unconditionally bound, and names_unbound flagged `return o` even though
`o` is bound on every path reaching the return.

A name is now guaranteed after a try statement when every fall-through
path binds it: the success path (try body + else) intersected with each
handler, plus anything finally binds. Without handlers an exception
propagates out of the function, so the success path alone counts.

Closes #285
has_parameterized_bound/has_parameterized_constraint were computed by
expr_is_parameterized, which flagged ANY subscripted type outside a
typing-forms whitelist — answering "is this generic?" rather than "is
this parameterized by a type variable?". PEP 484 permits concrete
generic bounds such as Callable[..., Any] or dict[str, int]; only type
arguments referencing a type variable (e.g. list[T]) are invalid.

The flags are now computed by expr_parameterized_by_typevar, which
recurses into the subscript's type arguments looking for an actual type
variable reference: a TypeVar declared earlier in the module, or a name
matching the single-letter uppercase convention. The obsolete helper
and its TYPING_FORMS whitelist are removed.

Closes #283
A legacy implicit alias with a structural container body, e.g.
`RefsDictT = dict[tuple[str, str], str]`, was collected by no alias map:
collect_union_aliases kept only Union-bodied definitions, and the
generic-alias map only takes TypeVar-parameterised bodies. The
annotation therefore stayed an opaque Named type, and the empty dict
literal (dict[Never, Never]) was reported as not assignable to it.

The collector (renamed collect_value_aliases) now also accepts concrete
dict/list/set/tuple definitions, excluding TypeVar-parameterised bodies
which remain the generic-alias map's job. The alias expands to its
structural type, where Never vacuously matches any target, so `{}` is
assignable — while genuinely mismatched literals (e.g. an int key
against tuple[str, str]) still fire after expansion.

Closes #282
The blank and caret-underline gutter rows padded the pipe two columns
further right than the source row, so every underline rendered shifted
two columns off the flagged code (issue #279). All rows now use the
rustc-standard width+1 gutter.
A base recorded as an attribute of an unresolved module (class
Client(httpx.Client)) resolves by simple name back to the class itself,
making the class its own ancestor. Twelve rule/resolver walks recursed
on that chain without a cycle guard and overflowed the stack, aborting
the LSP in a restart loop on large workspaces such as Apache Airflow
(issue #278 — the earlier 64 MiB analysis stacks cannot save an
infinite recursion).

Predicate-shaped walks now share two cycle-safe helpers in
rules/shared.rs (class_or_base_matches, any_base_name_matches); base
names pass through verbatim so each call site keeps its exact
normalisation and nothing changes but termination. Accumulator and
first-match walks in constructors_call_init, dataclasses_transform_class
and the resolver's protocol_ext carry inline visited sets.

Regression test runs the checker on the minimal airflow shape and also
pins the follow-on false positive (a class counting as overriding its
own methods through the name collision).
…_ hint

Three hover gaps, one shared cause — member information never survived to
the hover handler:

- issue #287: `Model.model_validate(...)` on a pydantic BaseModel subclass
  showed nothing. Class methods were dropped at every layer: ExternalSymbol
  had no member field, stub/py.typed export extraction discarded them, and
  runtime-generated stubs emitted bodyless `class X: ...`. All three layers
  now carry methods, and hover resolves dot-access through local base
  chains (cycle-guarded) into imported classes.

- issue #288: `" ".join(...)` showed nothing. New curated typeshed mirror
  `basilisk_stubs::builtin_method_signature` covers all public str methods;
  hover types string-literal receivers (with a bytes-prefix guard) and
  str-annotated/inferred variables.

- issue #289: class hovers now append the constructor signature — the
  class's own `__init__`, else the nearest inherited one in the local base
  chain.

The three lookups share one breadth-first, cycle-guarded base walker.
hover.rs (834 lines) is split into hover/{mod,members,tests}.rs to meet
the 500-LOC rule.
Stopping a CPU profile previously opened the raw .cpuprofile in VS Code's
built-in viewer — a wall-of-numbers self/total-time table — while the
Basilisk Profiler results panel hid behind a dismissable toast action.
Now the self-contained results panel (summary cards, flame graph hero,
click-to-source hot tables) opens automatically on stop, and the native
trace stays one deliberate click away: the completion toast's "Open Trace
in VS Code Viewer" action, the panel's button of the same name, or the
new "Basilisk: Show Profile Results" palette command that re-opens the
last result. Snapshot toasts carry a "View Results" action. An explicit
native-viewer request that cannot open is said out loud and reveals the
trace file instead of failing silently.

Also resolves the [PROFILE-UI-GATE] spec/code contradiction: profiling
now ships enabled by decision, not by an accidentally-committed override.
profiling-ui.ts is deleted, every basilisk.profilingEnabled manifest
reference is removed (session-state gating such as basilisk.memoryTracking
is kept), the spec section is rewritten to document the shipped state,
and a manifest sweep test fails if the dead context key ever returns —
a when clause on a key nobody sets would silently hide the UI from every
shipped user. The branch-audit conform-5 item is recorded as signed off.

Verified: full VS Code extension suite 505 passing; the real CPU e2e
asserts the panel opens with zero clicks on an actual debug-launched
session and stays reachable after closing.
The page had drifted badly from the implementation and described several
features that never existed. Audited every claim against the extension
sources, the LSP crates, and LSP-PROFILING-SPEC.md, then rewrote the
English page and its Chinese mirror.

Removed fabrications: the [tool.basilisk.profiler] pyproject section
(profiler config is editor settings only), the CPU "Compare Profiles"
command, BSK-PROF-GIL detection, percentage-based leak confidence
scores, the basilisk-profiles/ output directory, and the Linux "child
shim".

Corrected facts: heat thresholds are 20/10/5/1 (not 10/5/2/1); profiler
diagnostics are Hints and memory codes are BSK-MEM-* with per-code
severities (BSK-PROF-MEM never existed); real VS Code command titles;
Zed slash commands are informational and /memsnap //memdiff do not
exist; Neovim commands are :BasiliskProfile/:BasiliskMemLeak/
:BasiliskMemStop/:BasiliskMemRefs; artifacts go to the OS temp dir;
snapshots write no files; memory profiling requires a debug session.

Added the current UX: the Python Processes panel as the primary entry
point, the results panel opening on stop with Show Profile Results
re-entry, native .cpuprofile/.heapprofile viewers, the cooperative
macOS sampler, memory autopilot, final snapshot on exit, the
too-short-to-sample message, and the full 12-setting configuration
table. The competitor table is reduced to sourced, linked claims per
the documentation-honesty rule.
"Open in Speedscope" previously revealed the trace file and dropped the
user on speedscope.app's empty Browse page for a manual drag-in, because
an https page can never read file:// URLs. Browsers do treat loopback as
a potentially-trustworthy origin, so a new in-extension HTTP server
(profile-server.ts) serves each exported profile over
http://127.0.0.1:<port>/<token>/<basename> and the speedscope deep link
loads it automatically. Containment: loopback-only bind, unguessable
32-hex token per file, GET only, no-store, 10-minute registration
expiry, torn down with the extension. A companion toast keeps the
reveal-and-drag path for browsers without the loopback mixed-content
exemption, and a serving failure falls back to it directly.

Memory profiling gets the same landing inversion as CPU: taking a
snapshot now opens the Basilisk memory dashboard instead of the raw
.heapprofile table (presentSnapshot's option renamed to
openResultsView). The dashboard offers the raw trace on demand via two
new buttons — "Open Heap Profile in VS Code Viewer" and "Open in
Speedscope (external)" (speedscope imports V8 sampling heap profiles) —
both rendered only when a .heapprofile was written, both routed through
the shared openNativeTraceViewer / openSpeedscopeImport helpers. URLs
carry the real basename so speedscope's extension-based format
detection distinguishes heap profiles from speedscope JSON.

Spec sections [PROFILE-VIEWER-DELIVERY], [PROFILE-NATIVE], and
[PROFILE-MEMORY-FINAL] rewritten to match; website profiler docs
(en + zh) updated. New profile-server.test.ts pins the served headers,
token access control, expiry, and teardown; the dashboard-button and
webview suites extended; full extension suite green.
The quick-start's profiling step (en + zh) described a process picker
that never existed and the old flow: it now walks the Python Processes
panel launches, the auto-opening results panel, the real Stop keybinding
chord, the debug-session requirement for memory tracking, and the real
"Compare Memory Snapshots" command title; the "profile diffing" promise
is gone. The Zed install page's slash-command table oversold the
profiling/memory commands as actions — they are guides that explain the
matching basilisk.profiler.* / basilisk.memory.* LSP commands, and the
table now says so. The docs index (en + zh) blurb no longer attributes
all CPU profiling to py-spy, since debug-launched macOS sessions use the
cooperative in-process sampler.
cargo fmt --all --check flagged seven files with formatting drift from
recent work; applied. Coverage ratchets move up to measured minus the
1% buffer: basilisk-cli 89 -> 90 (measured 91), vsix 91 -> 92
(measured 93). Full local CI checklist green at these levels: lint
(clippy + eslint + deslop), Zed (wasm build, clippy, 97 tests, mirror
standalone build), website (rules.json sync, conformance refs, build,
24 Playwright tests), Rust tests + conformance gate + all crate
thresholds, Neovim (43% >= 42%), VS Code e2e suite (93% >= 91%).
…merge

Post-merge coverage measured basilisk-cli at 95% and basilisk-stubs at
93%; thresholds move to measured minus the 1% buffer (94 and 92). Full
local CI checklist re-run green on the merged tree: format, lint
(deslop 12.53% < 12.6% with the current CLI), Zed (all four steps),
website (build + 24 e2e), Rust tests + conformance gate + all
thresholds, VS Code (510 passing, 93% >= 92%), Neovim (43% >= 42%),
Shipwright binary gate, release build, and mutation testing (125
mutants, 118 caught, 0 missed, 100% kill rate, baseline unchanged).
@abdushakoor12
abdushakoor12 merged commit 249cdee into main Jul 8, 2026
22 checks passed
@abdushakoor12 abdushakoor12 linked an issue Jul 8, 2026 that may be closed by this pull request
@abdushakoor12
abdushakoor12 deleted the nimble/vibrant-hopper-8364f3 branch July 8, 2026 17:49
abdushakoor12 added a commit that referenced this pull request Jul 9, 2026
…ber hover (#278, #287) (#299)

## TLDR
Fixes the LSP stack-overflow restart loop on self-named external bases
(#278) and makes external-member hover actually work — pydantic's
`model_validate` now hovers on real packages (#287).

## What Was Added?
- `chase_py_typed_reexport` / `sibling_module_file` in
`crates/basilisk-checker/src/exports.rs`: when a `from pkg import Name`
lookup misses in a py.typed package's resolved file, the population pass
now follows the `__init__`'s module-level named **and star** re-export
imports (including inside `if TYPE_CHECKING:` blocks) into sibling
module files and takes the symbol — with its methods — from there.
Cycle-guarded via a visited set; runs only on a miss, only for py.typed
sources.
- Regression tests:
- `self_named_external_base_constructor_call_does_not_overflow`
(checker) — the `class Client(httpx.Client)` + `Client()` shape that
SIGABRT'd the process.
- `test_hover_on_method_reexported_through_py_typed_package_init`
(hover) — an on-disk pydantic-v2-shaped package: `__init__.py`
containing only `if TYPE_CHECKING: from .main import *`, `main.py`
defining the class with a `@classmethod`.
- `default_mode_analysis_populates_external_imported_symbols` (salsa
engine) — default-mode analysis must carry external `imported_symbols`.
- `benchmarks/status/darwin-arm64-apple-m4.csv` — first bench baseline
for this machine, per the trend policy.

## What Was Changed or Deleted?
- **`constructors_call_init` cycle guards (#278):**
`find_init_in_hierarchy`, `has_custom_init_in_bases`, and `is_subclass`
walked base-class chains with no cycle guard. A class named after its
unresolved external base (`class Client(httpx.Client)`) resolves by
simple name back to itself in the class map, so constructing it recursed
to a stack overflow — SIGABRT, LSP restart loop, unusable editor. All
three walks now thread `visited` sets, the same pattern as the twelve
walks guarded in #298. Behaviour is unchanged except termination.
- **Salsa engine wiring (#287):** `SalsaAnalysisEngine::analyse`
populated `imported_symbols` only in `crossModule` analysis mode — which
is documented "reserved for future use" and never the default, so the
external-member hover shipped in #298 was dead in every real editor
session (its unit test injected the symbols by hand). The resolved view
handed to hover/completion/navigation now always comes from the memoized
`cross_resolved_module` query; **diagnostics remain mode-gated exactly
as before**, so non-cross modes keep byte-for-byte CLI parity on what
they report. Doc comments updated to match.
- Mutation baseline re-dated by the fresh run (identical 100% score,
same 125-mutant pool).

## How Do The Automated Tests Prove It Works?
- `self_named_external_base_constructor_call_does_not_overflow` aborted
the test process with `stack overflow / SIGABRT` before the fix and now
passes, pinning both termination and the no-false-positive behaviour
(`constructors_call_init` must not fire).
- `test_hover_on_method_reexported_through_py_typed_package_init` failed
at `imported_symbols.contains_key("BaseModel")` before the extraction
fix (and again when the fixture switched to the real star-import shape
before the star chase existed); it now asserts hover renders
`BaseModel.model_validate` from the re-exported class.
- `default_mode_analysis_populates_external_imported_symbols` failed
with empty `imported_symbols` before the engine change; it now proves
default-mode analysis populates them while the diagnostics path is
untouched.
- Verified end-to-end over raw LSP against real pydantic 2.13.4: hover
on `model_validate` returns `(method) def BaseModel.model_validate(cls,
obj: Any, …) -> Self`; `basilisk check` on the crashing repro file went
from exit 134 (SIGABRT) to a clean run.
- Full local CI pass: lint (fmt/clippy/eslint/deslop), workspace tests +
coverage thresholds (all 10 crates), conformance **141/141 = 100%**
against freshly fetched `python/typing@main` fixtures, VS Code E2E (510
passing against the staged release VSIX), Neovim (43% ≥ 42%), Zed (97
tests), Shipwright version contracts, mutation testing (125 mutants, 118
caught, 0 missed, 100% kill rate, baseline unchanged), `make bench`
PASS, release build.

## Spec / Doc Changes
None — code-level doc comments only (`salsa_engine.rs` module/method
docs now describe the always-cross resolved view; the new exports
helpers document the re-export chase).

## Breaking Changes
- [x] None
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.

False positive "returns o but o may be unbound on some paths"

1 participant