Skip to content

Six bug fixes: hover inference, glyph tallies, safe-only Fix All, uv import isolation, Modules loading state, deep-expression stack overflow#294

Merged
abdushakoor12 merged 8 commits into
mainfrom
nimble/peaceful-newton-281549
Jul 7, 2026
Merged

Six bug fixes: hover inference, glyph tallies, safe-only Fix All, uv import isolation, Modules loading state, deep-expression stack overflow#294
abdushakoor12 merged 8 commits into
mainfrom
nimble/peaceful-newton-281549

Conversation

@abdushakoor12

Copy link
Copy Markdown
Collaborator

TLDR

Six user-facing bug fixes across the LSP, CLI, parser, and VS Code extension: hover type inference for unannotated functions, glyph-styled module tallies, safe-only Fix All defaults, uv-locked import isolation, an honest Modules-panel loading state, and a two-layer defence against the deep-expression stack overflow that crash-looped the server.

Closes #253
Closes #236
Closes #245
Closes #252
Closes #144
Closes #278

What Was Added?

  • crates/basilisk-lsp/src/runtime.rs (new, implements [LSPARCH-ARCH-STACK]): every production entry point now runs analysis on 64 MiB stacks — run_with_analysis_stack hosts the CLI's work thread, and block_on_with_analysis_stack builds the tokio runtime (block-on thread + workers both sized) for the stdio and WebSocket LSP servers. Bare Runtime::new() is gone from production paths (Stack overflow on VS Code when opening a large project #278).
  • Operator-chain guard in crates/basilisk-parser/src/depth.rs (extends [CHKARCH-ARCH-PARSEDEPTH]): the linear pre-parse lexer scan now counts depth-building tokens (binary/unary operators, ., ternary, lambda) per bracket level and rejects chains past 50,000 with "expression too deeply nested" — the construct the spec previously documented as a known residual. Flat-by-construction operators (and/or, comparisons, commas) are exempt so giant generated literals still analyse.
  • RhsKind::KnownCall in the resolver: two-argument <dict-literal>.get(key, default) calls resolve to their common value/default type, so hover infers -> str for the Hover shows no inferred types for unannotated function params/return (def f(missed, caught, ...)) #253 repro. Display-only — no diagnostic or conformance change.
  • Safe/All fix-all command split ([AUTOFIX-CLASSIFY]): four commands (fixAllSafe/fixAll × file/workspace) in basilisk-common, the LSP command handlers, and the VS Code palette. source.fixAll and the default palette action apply only SAFE_FIXABLE_RULES; BSK-E0003's : Any fix is unsafe-tier only ([AUTOFIX-CLASSIFY] LSP fix-all surfaces apply the Unsafe BSK-E0003 fix by default; promised Safe command variants don't exist #245).
  • scanComplete flag on the LSP's module-stats payload plus a basilisk/scanComplete notification, consumed by the extension store (Modules panel falsely shows "No Python files found" while the analyzer is still starting up #144).
  • Shared real-binary LSP test harness crates/basilisk-cli/tests/lsp_stdio/mod.rs, hoisted from e2e_lsp_no_ruff.rs and reused by the new deep-expression suite.

What Was Changed or Deleted?

How Do The Automated Tests Prove It Works?

  • e2e_deep_expressions.rs (real binary over stdio): workspace_scan_survives_deeply_chained_binary_expression opens a workspace whose file is a 10,000-term 1 + 1 + … chain — the pre-fix server aborts with stack overflow; the test requires basilisk/scanComplete plus a hover response. cli_check_survives_deeply_chained_binary_expression checks 30,000 terms exits 0, and cli_check_rejects_pathological_expression_depth_instead_of_crashing requires 300,000 terms to be skipped with "expression too deeply nested" rather than crash.
  • parse_tests.rs pins the guard boundary: 50,000 operators parse, 50,001 report the message, comma-separated flat sums and bracket-isolated sub-chains stay accepted, unbalanced closers don't disarm the counter, and 60,000-deep unary/attribute/ternary/lambda chains are all rejected.
  • lsp_e2e_hover.rs::test_lsp_hover_unannotated_function_shows_inferred_types asserts the Hover shows no inferred types for unannotated function params/return (def f(missed, caught, ...)) #253 repro hovers as (function) def extracted_function(missed: Unknown, …) -> str.
  • lsp_e2e_code_actions.rs asserts the source.fixAll surface excludes the unsafe BSK-E0003 edit while the explicit all-fixes command includes it.
  • module-explorer-loading.test.ts walks the Modules panel falsely shows "No Python files found" while the analyzer is still starting up #144 lifecycle: pre-scan state renders "Analyzing workspace…", post-scanComplete empty workspace renders "No Python files found"; module-explorer-tree.test.ts and activity-panel.test.ts assert the 🔴/🟠 glyph format and zero-severity omission.
  • Full local CI matrix on this exact head: conformance 100 % (141/141, 0 false positives), all per-crate coverage thresholds met (parser 100 %), mutation testing 125 mutants / 0 missed / 100 % kill rate matching baseline, Zed 97 tests, website 24 e2e, shipwright version contracts valid, release build clean under -D warnings.

Spec / Doc Changes

  • LSP-ARCHITECTURE-SPEC.md: new [LSPARCH-ARCH-STACK] section — analysis stack sizing, the entry-point contract (never bare Runtime::new()), cross-reference to the parser guard.
  • CHECKER-ARCHITECTURE-SPEC.md: [CHKARCH-ARCH-PARSEDEPTH] gains the operator-chain limit (replacing the "known residual" note) with counted tokens, reset tokens, and exemption rationale.
  • LSP-MASS-AUTOFIX-SPEC.md ([AUTOFIX-CLASSIFY]): Safe vs Unsafe tier definitions and the four-command surface.
  • LSP-UV-INTEGRATION-SPEC.md ([LSPUV-DIAGNOSTICS-MODULE-NOT-FOUND]): uv-locked resolution contract.
  • EXTENSION-ACTIVITY-PANEL-SPEC.md: [EXTACT-MODULES-COUNT-STYLE] glyph format and [EXTACT-MODULES-HEADER] scan-gated empty state.
  • vscode-extension/README.md + regenerated vscode-module-explorer.png screenshot reflecting the glyph tallies.

Breaking Changes

  • None — the existing basilisk.fixFile / basilisk.fixWorkspace commands keep their IDs but now apply safe fixes only (the intended default per [AUTOFIX-CLASSIFY]); the previous unsafe-inclusive behaviour remains available via the new explicit basilisk.fixFileAll / basilisk.fixWorkspaceAll commands.

Hover on an unannotated function previously rendered a bare, type-less
signature. Now:

- The return type is inferred from the body's return statements and
  rendered when the annotation is missing (shared with inlay hints via
  the hoisted util::infer_return_type_display).
- Unannotated parameters render ': Unknown' instead of blank, except
  the implicit self/cls receiver.
- classify_rhs resolves two-argument <dict-literal>.get(key, default)
  calls to a new RhsKind::KnownCall carrying the common value/default
  kind, so the issue's repro infers '-> str'. The checker maps
  KnownCall exactly like CallExpr — display-only, no diagnostic or
  conformance change.

Adds a stdio LSP e2e regression test on the exact repro and documents
the inferred-display behavior in [LSPARCH-FEATURES-HOVER].
…LES-COUNT-STYLE] (#236)

diagnosticTally emitted the forbidden lettered `nE nW` form on all three
Modules-panel surfaces (module rows, folder/package rollups, workspace
header message). Switch it to the spec-mandated coloured glyph tally,
omitting zero severities. Adds a regression e2e test, flips the five
existing assertions that encoded the old format, restores the regressed
[EXTACT-MODULES-TREE-STRUCTURE] spec bullet, and regenerates
vscode-module-explorer.png via the sanctioned screenshot pipeline.
…ASSIFY] (#245)

All three LSP fix-all surfaces (source.fixAll code action, basilisk.fixFile,
basilisk.fixWorkspace) applied the unfiltered ALL_FIXABLE_RULES set, silently
including the Unsafe BSK-E0003 `: Any` insertion, and the Safe/All command
split promised by [AUTOFIX-MASS-VSCODE] did not exist. Route the default
surfaces through fix_filtered_in_file(SAFE_FIXABLE_RULES) — the machinery the
CLI already uses — and add explicit all-tier basilisk.fixFileAll /
basilisk.fixWorkspaceAll commands (constants, dispatch, VS Code palette
entries, URI-injection middleware). Adds a stdio e2e regression test covering
all five surfaces, updates the [AUTOFIX-CONFLICTS] honest-state note and the
retitled Fix All (Safe) references in the activity-panel spec and README.
…nterpreter (#252)

The lockfile E0010 tests were not hermetic: a uv-locked project with no venv
fell back to `python3` sys.path discovery, so `import flask` resolved on any
machine whose first ambient site-packages carried flask and `make test` went
red (green machines passed only by sys.path ordering luck). Gate the
ambient-interpreter fallback off when a PackageRegistry exists — the lock is
the source of truth, so a dependency missing from it now diagnoses identically
on every machine. In-tree venv discovery and an explicitly activated
VIRTUAL_ENV (issue #25) still apply to locked projects. States the lock-only
resolution contract in [LSPUV-DIAGNOSTICS-MODULE-NOT-FOUND] and adds a
deterministic regression test at the search_paths_from_config seam.
…the scan finishes (#144)

The Modules panel rendered the terminal 'No Python files found' empty-state
whenever the workspace rollup reported totalFiles == 0 — including the whole
window between the server reaching Running and its background workspace scan
finishing, and it showed nothing at all while the client was still starting.

Server: LspServer tracks initial_scan_complete; every basilisk.workspaceModules
rollup stamps scanComplete, and a new basilisk/scanComplete notification fires
when a scan finishes (set immediately in openFilesOnly, where no scan runs).
The notification is required, not an optimisation: a genuinely empty workspace
publishes no diagnostics, so nothing else would settle the loading state.

Client: workspaceHealthMessage renders 'Analyzing workspace…' when no stats
exist yet or totalFiles == 0 without scanComplete; the #57 empty-state now
requires a finished scan. The store bumps analysisRevision on scanComplete, so
the repaint flows through the existing signals effect — no polling, no
panel-local readiness flags.

Spec: [EXTACT-MODULES-HEADER-LOADING] loading clause,
[EXTACT-LSP-COMMANDS-SCAN-COMPLETE] notification, HealthStats.scanComplete in
both wire-shape sections; e2e regression tests drive the real provider through
the tree-view chrome for the mid-scan, pre-ready, and genuinely-empty states.
…shes analysis (#278)

A single generated file like `total = 1 + 1 + …` parses flat but builds an
arbitrarily deep left-nested BinOp tree; the recursive resolver/checker
visitors then overflowed the default ~2 MiB tokio worker stack during the
workspace scan, aborting the whole LSP server and crash-looping the editor's
restart logic (0xC00000FD / 'tokio-rt-worker has overflowed its stack'). The
CLI had the same exposure on the process main thread.

Two complementary mechanisms per [LSPARCH-ARCH-STACK] and the extended
[CHKARCH-ARCH-PARSEDEPTH]:

- Every thread that can run analysis now has a 64 MiB stack: the new
  basilisk-lsp runtime module hosts block_on and all tokio workers on
  analysis-sized stacks for both LSP entry points, and the CLI dispatches
  every subcommand through its synchronous counterpart.
- The parser's linear pre-parse nesting guard (previously brackets and
  indentation only — its spec listed this construct as a known residual)
  now also caps depth-building operator chains at 50,000 per expression
  context, rejecting pathological files with 'expression too deeply nested'
  before any recursion can start. Flat constructs (and/or, comparisons,
  comma lists) are exempt so giant generated literals stay analysable.
  Limits sit >2x under the measured 64 MiB crash floor (~150k).

Tested with real-binary e2e coverage: the LSP scan survives and stays
responsive on a 10,000-term chain, basilisk check analyses 30,000 terms
cleanly (previously SIGABRT), 300,000 terms is skipped with a warning like
the bracket cap (previously SIGABRT), plus operator-chain boundary tests in
the parser suite. The stdio LSP test harness moved to a shared module. Bench
gate passes with no regression; conformance holds at 100% (141/141), FP 0.
… [PROFILE-HELPER-PROTOCOL-ERRORS]

A helper exiting between the attach payload and newline writes hits the
sender as EPIPE, not the reader as EOF; the send path bailed with the
bare IO error instead of reaping the helper and surfacing its exit
status and stderr (#81 contract, raced on slow CI runners). Also joins
four lines that rustfmt 1.96 (CI stable) formats differently than 1.95.
@abdushakoor12
abdushakoor12 merged commit 3ee893e into main Jul 7, 2026
22 checks passed
@abdushakoor12
abdushakoor12 deleted the nimble/peaceful-newton-281549 branch July 7, 2026 10:22
MelbourneDeveloper added a commit that referenced this pull request Jul 7, 2026
The main merge (PR #294) brought code below three crates' coverage
thresholds. Add behavioral tests to restore them; thresholds unchanged:

- basilisk-parser: unit tests for depth::check_nesting (bracket/indent/
  operator-chain limits + boundaries, dedent, bracket-resume,
  chain-breakers) — depth.rs 45% -> 100%.
- basilisk-resolver: function-body TypedDict tests exercising the
  recursive check_td_stmts (del of required/NotRequired keys, reads,
  subscript-assign, disallowed mutator methods) — 94% -> 95%.
- basilisk-cli: stub-subcommand tests (find_package_source success,
  cache_stub, run_stubs* paths) — 87% -> 91%.

Authoritative scripts/test-rust.sh: all coverage thresholds pass,
conformance 141/141 (100%, 0 false positives).
MelbourneDeveloper added a commit that referenced this pull request Jul 7, 2026
…esh (+ main merge) (#295)

## TLDR
Consolidation of the `cleanup` branch — expands the benchmark-fixture
suite, splits `FsCache` into its own module (500-LOC cap), refreshes the
READMEs/website, ratchets the deslop duplication gate to 12.6% and
simplifies its installer — with current `main` merged in and every
per-crate coverage threshold restored.

## What Was Added?
- **12 benchmark fixtures** under `benchmarks/fixtures/*.py` (~24k
lines) expanding the perf suite (aliases, callables, constructors,
dataclasses, enums, generics defaults, literals, narrowing, overloads,
protocols, returns, tuples).
- **`crates/basilisk-checker/src/imports/fs_cache.rs`** — `FsCache`
extracted into its own module so `resolve.rs` stays under the 500-LOC
cap; its tests move with it.
- **Coverage tests** restoring three crates that the `main` merge
dropped below threshold:
- `basilisk-parser`: unit tests for `depth::check_nesting`
(bracket/indent/operator-chain limits + boundaries, dedent,
bracket-resume, chain-breakers).
- `basilisk-resolver`: function-body `TypedDict` tests exercising the
recursive `check_td_stmts` (required/`NotRequired` key deletes, reads,
subscript-assign, disallowed mutator methods).
- `basilisk-cli`: stub-subcommand tests (`find_package_source`,
`cache_stub`, `run_stubs*`).
- **`examples/cpu_demo_loop.py`** demo.

## What Was Changed or Deleted?
- **FsCache split**: `resolve.rs` imports `FsCache` from `fs_cache`;
`apply.rs` imports it from `super::fs_cache`.
- **Checker rule refinements**: `generics_type_erasure`,
`callables_kwargs`, `specialtypes_never_2`, `generics_basic_3/helpers`
precision changes.
- **Flaky DAP test hardened**: `debug_spawn.rs`
`a_missing_interpreter_fails_fast_without_port_retries` uses 3 candidate
ports so the documented free-port TOCTOU can't surface `PortTaken`
before the spawn (assertion unchanged).
- **Deslop gate ratcheted**: `.deslop.toml` `max_duplication_percent` →
12.6 with non-product excludes; `scripts/install-deslop.sh` reduced to
an unpinned `brew install nimblesite/tap/deslop`.
- **README refresh**: concise `vscode-extension/README.md`; root
`README`/`README.zh`; website conformance/benchmark data.
- **Merge of `main`** (PR #294: hover inference, glyph tallies,
safe-only Fix All, uv import isolation, Modules loading state,
deep-expression stack-overflow guard).

## How Do The Automated Tests Prove It Works?
- **Conformance** (`conformance/score.py --gate`): **141/141 (100%), 0
false positives, 0 missed required errors** against `python/typing@main`
(`f4f2952`).
- **Per-crate coverage** (`scripts/test-rust.sh`, thresholds in
`coverage-thresholds.json` **unchanged**): all pass — `basilisk-parser`
100%, `basilisk-resolver` 95%, `basilisk-cli` 91%, `basilisk-checker`
94%, plus db/lsp/mojo/parser/plugin/stubs/config.
- **Lint**: `cargo fmt --all --check`, `cargo clippy --workspace
--all-targets -- -D warnings`, and the deslop gate (repo-wide
duplication under the 12.6% ceiling) all clean.
- **Flaky fix**: the `debug_spawn` suite passes across repeated runs
that previously flaked under port contention.
- **Website**: `gen_conformance_reference.py --check` and
`gen_rules_reference.py --data` drift checks clean; 24 Playwright
navigation/screenshot e2e tests pass.

## Spec / Doc Changes
- `docs/specs/CHECKER-ARCHITECTURE-SPEC.md` conformance stamp updated to
the graded commit.
- READMEs (`README.md`, `README.zh.md`, `vscode-extension/README.md`)
and website docs refreshed.
- `.deslop.toml` duplication budget documented (ratchets down only).

## Breaking Changes
- [x] None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment