Repository navigation
Conversation
# Conflicts: # .rubocop_todo.yml
Applies the exact diff CI's `.rubocop_todo.yml` job computed (Ruby 3.4, rubocop 1.80.0) so the checked-in todo file matches the codebase's actual current offense state.
This PR previously targeted v0.59; retargeting onto master. Squashed the fork's net contribution (previously spread across the branch's merge-heavy history against v0.59) into a single commit applied cleanly on top of current castwide/master, resolving the one real conflict in .github/workflows/plugins.yml (master's own bundler-cache: false vs. this fork's bundler-cache: true perf fix for run_solargraph_rails_specs).
Workspace#resolve_require no longer exists on master (that logic now lives on Workspace::Gemspecs, per castwide's own refactor/revert history). Route through Workspace::Gemspecs directly, matching the pattern already used in spec/workspace/gemspecs_resolve_require_spec.rb. Fixes the two api_map_method_spec.rb failures the "regression" CI job caught (YAML/Psych and Thor.desc method-stack specs) - these were masked as "pre-existing" in earlier local testing because that comparison only checked before/after within the v0.59-based branch, not across the base-branch move to master where this method moved.
Root cause (two compounding bugs, both introduced by this branch's own earlier perf work, not upstream castwide code): 1. Workspace::Gemspecs#gemspec_or_preference returned whatever spec type it was given (Gem::Specification, Bundler::LazySpecification, or Bundler::StubSpecification) without normalizing via to_gem_specification, despite its own @return [Gem::Specification] contract. ApiMap#resolve_require (which funnels through this method) was therefore returning Bundler::StubSpecification objects that don't == the Gem::Specification objects DocMap's own uncached_yard_gemspecs/ uncached_rbs_collection_gemspecs tracking uses - so DocMap#cache's `uncached_yard_gemspecs.include?(gemspec)` check silently failed and cache_gem became a no-op. 2. spec/api_map_method_spec.rb's YAML and Thor tests called resolve_require + cache_gem *before* catalog(bench) - but catalog is what registers a gem as required in doc_map's internal tracking in the first place, so calling cache_gem first meant doc_map didn't yet know the gem needed caching. Reordered to catalog first. The YAML test happened to keep passing throughout because it's stdlib, cached via a separate always-on pathway (Ruby core RBS caching), masking both bugs for that case. Verified: reproduces and is fixed under both rbs 4.0.1 and 4.1.1; full local suite (bundle exec rake spec) is 1616 examples, 0 failures.
Found while verifying this PR's original claimed accomplishments are still intact. Same root cause as the Thor.desc fix: cache_gem was called before catalog(bench) registered 'yard' as required, making the cache a no-op. Confirmed via isolated fresh SOLARGRAPH_CACHE: failed before this fix, passes after. It was masked in full-suite runs by another spec warming yard's cache first in the same process - not currently causing CI failures, but the same latent landmine.
Rebases castwide#1167 (apiology/parallel_rspec) onto castwide/master instead of v0.59, since master and v0.59 have diverged substantially and v0.59 carries unrelated changes. This commit is the net diff of that branch (plus its merge-conflict and CI-regression fixes) applied directly against master; history was not preserved per request. Highlights: - Parallelize per-gem YARD/RBS pin caching with a thread pool (doc_map.rb, shell.rb) instead of caching gems serially. - Fix a mutex re-entry deadlock in Library#sync_catalog when the next cacheable gemspec is already being processed elsewhere (castwide#1220), with its regression test. - Fix an exponential-blowup bug in Pin::Method#combine_same_type_arity_signatures (O(n^2) bail-out for large signature sets), with its regression test. - shell.rb's gems 'core' command called PinCache.core?/PinCache.cache_core, which never existed; use the real Solargraph::RbsMap::CoreMap#pins API instead. - Bundler::LazySpecification#materialize_for_installation is an internal, undocumented Bundler API whose arity changed without a deprecation path; guard against all known shapes (modern wrapper, old zero-arg method, incompatible-arity method) instead of assuming one signature. - Misc RuboCop/YARD-doc fixes and .rubocop_todo.yml updates.
…reformat - ApiMap#resolve_require: raise a clear error when called without a workspace instead of suppressing the nil-workspace typecheck warning with @sg-ignore. - .github/workflows/rspec.yml: revert the undercover job's "Update types" step back to a single-line `run:` - the block-scalar form had identical content, a no-op reformat.
Investigated why CI explicitly pinned `bundler: 2.5.23` (matching Gemfile.lock's BUNDLED WITH) in both the main rspec matrix and undercover jobs. This pin is fork-only - never merged to castwide/master - introduced in 9d26863 "Fix new bundler issue" to work around a failure specific to the `ruby-version: head` matrix entry, which has since been removed entirely (unrelated 404 issue on ubuntu-24.04, see the @todo above the matrix). castwide/master's own rspec.yml has never had this pin and its CI passes consistently (confirmed via recent successful runs). Also reproduced locally: `bundle install` with the latest published bundler (4.0.17, vs the pinned 2.5.23) against this project's Gemfile.lock completes cleanly with no lockfile changes. This brings rspec.yml back to an exact match with castwide/master, removing it from this PR's diff. If CI still passes here, the pin was dead weight from a since-resolved, no-longer-applicable issue.
Each of these explained a distinct class of intermittent CI failure observed across this PR's rspec matrix/parallel_tests jobs, and will only get more frequent as test parallelism increases. - Diagnoser: an uncaught exception during a background diagnosis (e.g. a file/directory disappearing mid-diagnosis, such as protocol_spec's around-block temp dir cleanup racing the async diagnoser thread) killed the thread before it reached the line that marks it fully stopped, so Host#fully_stop hung for its full 240-second timeout every time. Now rescues broadly around individual diagnoses and guarantees the fully_stopped flag is set via ensure regardless of how the thread's loop exits. - MessageWorker: stop() never signaled its condition variable, so a thread blocked in tick's wait() with an empty queue could never wake up to notice stopped? and exit - a permanently leaked thread. Also added fully_stopped? tracking (matching Diagnoser) and wired it into Host#fully_stopped?, which previously didn't wait on MessageWorker's thread at all. - PinCache#save wrote directly to the final cache path with no atomicity. Multiple parallel_tests workers (separate OS processes) racing to cache the same not-yet-warm gem for the first time could corrupt or truncate each other's writes on the shared cache directory. Now writes to a temp file and renames into place (atomic on the same filesystem). - Yardoc.cache invoked a bare `yardoc` command, relying on it being found via shell PATH - which fails for unbundled environments/subprocesses where it only exists inside the current bundle's own bin directory. Now resolves the actual executable via Gem.bin_path, independent of PATH. - rubocop_helpers_spec.rb's "custom version" test unconditionally removed the process-global RuboCop constant in its cleanup, even when its own version-swap had been a no-op (because something else in the process, e.g. protocol_spec.rb's top-level require, had already loaded the real gem first) - i.e. even when there was nothing to restore. That left RuboCop undefined for the rest of the process, cascading into failures in unrelated specs (library_spec, protocol_spec's formatting/environment handlers, rubocop_spec) whenever this spec happened to run first. Now only cleans up (and reloads the real version) when the swap actually took effect. Local full-suite run: 13 failures -> 2, both isolated/self-contained and already understood (rubocop_helpers_spec's version-swap doesn't work when rubocop was already required by something else first, and a pre-existing gem_pins_spec bug in this PR's own test content).
This reverts commit 0cd6610.
Yardoc.cache's "check cached, else build" was a classic check-then-act race: two OS processes (e.g. two parallel_tests workers, each caching the same not-yet-cached gem for the first time) could both see "not cached" and run `yardoc --db path` concurrently against the same .yardoc database directory, corrupting or truncating each other's output. This is very likely the actual cause of spec/pin/base_spec.rb's intermittent "deals well with known closure combination issue" failure and the strict_spec.rb Kramdown-constant failure in CI (both build/read a gem's YARD pins via this path) - my earlier PinCache#save atomic- write fix only covered Solargraph's own Marshal cache files, not this separate tool-managed directory. Wrap the build in a per-gem flock'd lock file, re-checking cached? after acquiring the lock (double-checked locking) so only one process actually builds a given gem; the rest wait for the lock and then reuse what the first process built instead of racing. This is the same mechanism a prior, incomplete attempt at this (Yardoc.processing?, referenced from Library#diagnose but never actually used to coordinate Yardoc.cache itself) was clearly reaching for.
This reverts commit 68561b2.
The two remaining intermittent failures (Kramdown constant in strict_spec.rb, 0 pins in pin/base_spec.rb) were never a cross-process caching race at all - reproduced deterministically locally with a cold cache, in complete isolation, no concurrency involved. My earlier PinCache atomicity and yardoc-locking fixes were solving a real but different problem than this one. Both tests called ApiMap#cache_gem(spec) for a gem before ever telling the ApiMap's DocMap that gem was needed (via #catalog with external_requires). DocMap#cache only builds pins for gemspecs in its own uncached_yard_gemspecs/uncached_rbs_collection_gemspecs lists, which are only populated from requires resolved during #catalog - so cache_gem was silently a no-op, and building only happened to succeed when something else had already warmed the gem's cache earlier in the same process (hence "intermittent", depending entirely on test/file run order and cache state, not timing). Fixed both to use the catalog -> cache_all_for_doc_map! -> catalog sequence already used correctly elsewhere in this same PR (see rbs_map/conversions_spec.rb's "with superclass pin for Parser::AST::Node" context): catalog first so DocMap learns about the dependency, cache_all_for_doc_map! to build it, catalog again to reload the ApiMap's pin store with the now-cached pins. Verified: both pass individually and together with ~/.cache/solargraph completely cleared beforehand (previously guaranteed to fail cold, pass only by accident once something else had warmed the cache). Full local suite with a cold cache: 1625 examples, 2 failures - both isolated, already-known, unrelated issues (rubocop_helpers_spec's version-swap test doesn't work when rubocop was already required by something else first, and a real RBS/YARD merge bug in gem_pins_spec.rb) - no other instances of this cache-ordering bug found anywhere else in the suite.
protocol_spec.rb's around block chdirs into a per-example temp directory and back, on the main thread, without holding Solargraph::CHDIR_MUTEX - the same mutex that Diagnostics::Rubocop#diagnose and the textDocument/formatting handler already use specifically because RuboCop::Runner internally chdirs (with a block) to read config files. An earlier fix in this series made the background diagnoser thread resilient to errors instead of dying on the first one (rescuing broadly, guaranteeing fully_stopped? via ensure), so it now keeps running background diagnoses - including RuboCop ones - for longer during a test run. That made it far more likely to have an active chdir in flight from that mutex right as protocol_spec's own (unsynchronized) chdir ran, which Ruby raises as "conflicting chdir during another chdir block" (visible in CI as a wave of Protocol example failures, e.g. "handles textDocument/definition"). Route protocol_spec's chdir calls through the same Solargraph::CHDIR_MUTEX so they can't overlap with RuboCop's.
An audit of the full diff against master (prompted by "are we dragging in v0.59 changes that weren't intended?") found several small items inherited from the original apiology/parallel_rspec branch history, predating this session's rebase: - Diagnostics::Base#diagnose and TypeCheck#diagnose gained a `workspace:` kwarg (commit "Spec performance fixes", a 33% local speedup) so TypeChecker.new could reuse an already-loaded Workspace instead of implicitly building a fresh one via Workspace.new(File.dirname(filename)) on every diagnose call. The kwarg was added but the one production call site, Library#diagnose (library.rb), was never updated to pass it, so the optimization was inert. Wire it through, and add the same kwarg to the other Diagnostics::Base subclasses (Rubocop, UpdateErrors, RequireNotFound) so the polymorphic call in Library#diagnose doesn't raise ArgumentError for reporters that don't use it. - Remove RbsMap::StdlibMap.possible_stdlibs: added, never called. - Remove duplicate/redundant YARD @PARAM comments added to ComplexType#qualify and .parse alongside the existing docs. - Revert a no-op reordering of Workspace#gemfile?/gemspec?/gemspec_files back to their master position; both locations are public, so this wasn't a visibility change, just unexplained churn. Left alone: a stray "@todo Missed nil violation" comment in source/chain.rb, and RuboCop-autocorrect-driven formatting diffs elsewhere in the branch (YARD/CollectionStyle, quote style) that predate this rebase and are needed to keep Overcommit clean.
The Hash{Array(String, String) => ...} -> Hash{Array, String, String
=> ...} docstring reformatting in api_map/constants.rb,
api_map/store.rb, doc_map.rb, source_map.rb, and the quote/block-style
cleanup in spec/source/chain_spec.rb weren't related to this PR's
stated purpose (parallel specs, concurrency fixes) - they were fixing
YARD/CollectionStyle and Style/StringLiterals offenses that a fresh
`rubocop --auto-gen-config` surfaces under the currently-installed
RuboCop/rubocop-yard versions but that master's own committed
.rubocop_todo.yml doesn't yet grandfather (a pre-existing drift, not
something this branch introduced).
Revert those files to master's content and add a scoped
YARD/CollectionStyle todo exclusion for the same 4 files, so this
branch stays green without carrying the unrelated reformatting. The
actual fix now lives in a standalone PR:
#42.
Two follow-ups to the earlier extraction: - The per-file YARD/CollectionStyle todo exclusion was a stand-in for actually fixing the cop. Replace it with disabling the cop outright in .rubocop.yml, matching castwide#1237's fix: for nested-generic/tuple Hash key types, the cop's own long-style autocorrect produces syntax that doesn't preserve the original tuple's meaning, so there's no safe autocorrected form to converge on for those cases. - UniqueType#to_rbs's all_params.empty? nil-safety fix is an unrelated, pre-existing bug (from the original branch's own history, unrelated to parallel specs) - move it to #43.
Rebases this branch's net contribution onto apiology/speed_up_specs_master (castwide#1237) instead of master directly, since both branches independently fixed the same catalog-before-cache_gem gem-pin-caching ordering bug and both added Diagnostics::Base#diagnose's workspace: kwarg. Resolving those as one shared fix (favoring castwide#1237's implementation, already under review upstream) instead of carrying two competing copies. Conflict resolutions: - ApiMap#resolve_require: kept castwide#1237's version (raises on a nil workspace, per its own review feedback) over this branch's safe-navigation version. - spec/api_map_method_spec.rb, spec/pin/base_spec.rb: kept castwide#1237's structure/helpers (`let(:catalog)`, before-hook pattern) where it already covers the same ordering fix; kept this branch's cache_all_for_doc_map! call in pin/base_spec.rb since it exercises the parallel-caching path this PR is actually about. - spec/rbs_map/conversions_spec.rb: kept castwide#1237's shared before(:all) ApiMap load across all examples in the file (faster than this branch's per-example ApiMap.new) - same test cases either way. - spec/yard_map/mapper_spec.rb: kept castwide#1237's removal of a test whose description ("marks correct return type from RuboCop::Options.new") no longer matched its body (had been repointed at Open3.capture2e, already covered by spec/rbs_map/conversions_spec.rb). - .github/workflows/rspec.yml: kept castwide#1237's pinned `bundler: 2.5.23` install step alongside this branch's `rake full_spec`/pre-caching additions.
- Bump dev-dependency RuboCop from 1.80.0 to 1.89.0: 1.80.0 had a bug
where per-file AllCops:Exclude entries didn't remove files from the
scan target list, so spec/fixtures/invalid_{byte,utf8,node_comment}.rb
(fixtures with intentionally invalid encoding, used to test
Solargraph's own error handling) always failed Lint/Syntax regardless
of Exclude config. 1.89.0 fixes this.
- Consolidate the three fixture exclude entries into one glob.
- Fix the todo-sync step: rubocop --auto-gen-config's exit code
reflects the total offense count, not just new offenses, so under
GitHub Actions' default bash -e it aborted before reaching the
git-diff check that's meant to be the actual pass/fail signal.
- Remove continue-on-error from both rubocop_todo steps and
regenerate .rubocop_todo.yml against 1.89.0.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018s7rWmpEoZpj4o3To7F286
The previously committed version was missing --no-offense-counts (78 stray "# Offense count: N" comment lines), causing the todo-sync check to fail: CI's fresh `bundle install` regeneration diverged from the committed file even with identical gem versions installed.
When rubocop is clean (the "Run RuboCop" step already passed), any diff from regenerating .rubocop_todo.yml can only be a shrink -- offenses that got fixed without removing their todo entry -- never a new offense. On push to master, commit and push that cleanup automatically instead of asking someone to do it by hand. This can't apply to PRs: GITHUB_TOKEN can't push to a fork's branch, and most PRs on this repo come from forks. PRs keep the existing fail-with-diff behavior.
GitHub moved ubuntu-latest to Ubuntu 26.04 starting 2026-10-19, and every Ruby job broke there before reaching repo code: - rugged 1.9.6 (needed by undercover) fails its native build at "checking for -lgit2" with "The compiler failed to generate an executable file" under GCC 15; rugged's own CI fails the same way on 26.04 (libgit2/rugged#1016). - ruby/setup-ruby has no Ruby 3.1 build for 26.04, since 3.1 does not compile with GCC 15 (ruby/setup-ruby#922). Neither is fixed by a newer setup-ruby: the failing jobs already ran v1.331.0. Name the runner image so CI stays on 24.04 until rugged builds on 26.04 and 3.1 leaves the matrix.
…' into parallel_rspec_on_1237 # Conflicts: # .github/workflows/plugins.yml
Solargraph resolves Benchmark::Tms#real to the benchmark gem's own source pin, which has no return type, ahead of the RBS pin declaring Float. That left the whole (time.real * 1000).to_i chain undefined, so CI reported the > comparison against SLOW_BATCH_MS as unresolved. Kernel#Integer returns Integer regardless of its argument's type.
setup-ruby's bundler-cache installs gems into ./vendor/bundle and writes .bundle/config in the checkout. The now-blocking .rubocop_todo.yml check fails on any `git status --porcelain` output, so those untracked directories failed it once both changes met.
The .to_i and Integer() rewrites only existed to silence CI's "Unresolved call to >" at the SLOW_BATCH_MS comparison, which is a Solargraph defect (benchmark's untyped Benchmark::Tms#real source pin shadows the RBS Float return type), not a problem in this code.
Benchmark::Tms#real resolves to the benchmark gem's source pin, which has no @return, ahead of the RBS pin declaring Float, so the comparison is reported as unresolved. Reproduced on Linux Ruby 4.0.7 with CI's Gemfile.lock, plugins and RBS collection; it does not reproduce on macOS Ruby 4.0.6.
castwide#1434 pinned CI runners to ubuntu-24.04 and was closed in favor of castwide#1436, which builds rugged on ubuntu-latest (26.04) instead. Back out its merge so castwide#1436 can come in through castwide/master. This reverts commit 1c030a7, reversing changes made to 1ac2f49.
…d auto-prune .rubocop_todo.yml
…ailed check without blocking merge
Resolve the .rubocop_todo.yml conflict by regenerating it with castwide#1260 todo-check command on Ruby 3.4.10, as CI does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEntZKsZwQHhzqcwG7MTqn
Keep the block-scalar run: for Check PR coverage and take castwide#1047 job-level continue-on-error, dropping the step-level one. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEntZKsZwQHhzqcwG7MTqn
…module_function methods
… methods with assertions on
… in class << self and Struct.new blocks
…s over their class, not the documented method
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.
Claude: This PR was written by Claude Code on behalf of @apiology.
Problem:
Open PRs are tested only one at a time:
A conflict between two of them would surface only after both merge.
Solution:
Merges every PR below into one branch so CI runs on the combination.
castwide/solargraph PRs included
Notes
Merge <repo>#Nmerge commit.