Rewrite PinCache as an instance-based engine, with wiring and CLI update - #35
Closed
apiology wants to merge 6 commits into
Closed
Rewrite PinCache as an instance-based engine, with wiring and CLI update#35apiology wants to merge 6 commits into
apiology wants to merge 6 commits into
Conversation
Introduce Solargraph::PinCache, replacing the old class-method-based PinCache module with an instance-based engine that owns YARD and RBS collection caching, plus combining them into a single cached pin set per gem. Yardoc, GemPins, and RbsMap are updated to support it: * Yardoc splits doc-building (build_docs/build_pins) out of its old do-everything cache method, so PinCache can drive the build and caching steps separately. * GemPins drops build_yard_pins (now owned by PinCache) and adds combine_method_pins_by_path for deduping method pins by path. * RbsMap falls back to StdlibMap resolution when a gemspec isn't found in the RBS collection. Also fixes a real bug in RbsMap::Conversions surfaced while extracting this: two pairs of duplicate method definitions (parts_of_function, build_type) where an old implementation was left in place, shadowed and made unreachable by a newer one added elsewhere in the file. The dead code referenced two helper methods (other_type_to_type, method_type_to_type) that don't exist anywhere in lib/, so it would have raised NoMethodError had it ever been called - removing it drops this file's strong-typecheck problem count from 32 to 21 (all pre-existing, unrelated to this change). Extracted from castwide#1006 (Improve pin caching) as the foundational piece of that PR: the new caching engine and its direct collaborators, without yet wiring it into DocMap/Workspace/ApiMap or the CLI (those follow in stacked PRs on top of this one). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace DocMap's ad hoc gem-caching logic with delegation to PinCache (introduced in the prior stacked PR), simplifying DocMap substantially. Workspace gains a pin_cache accessor plus cache_gem/uncache_gem/ cache_all_for_workspace! entry points that drive PinCache for a given workspace's gemspecs. ApiMap follows the renamed DocMap API (cache_all! -> cache_doc_map_gems!, uncached_gemspecs.any? -> any_uncached?) and dedupes resolved method aliases via GemPins.combine_method_pins_by_path. Library exposes pin_cache (delegating to workspace), uses it to check whether a gem's cache build is already in progress, and fixes a subprocess chdir bug in its background gem-caching thread. Extracted from castwide#1006 (Improve pin caching) as the second piece of that PR, stacked on top of the PinCache engine PR. This depends on PinCache existing; the CLI updates that depend on this wiring follow in a further stacked PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reimplement `solargraph cache`, `uncache`, and `gems` as thin wrappers
over the Workspace#cache_gem/uncache_gem/cache_all_for_workspace!
entry points added in the prior stacked PR, removing the CLI's own
duplicated build/cache logic.
Note: 2 specs in this PR ("with unbundled environments #cache
succeeds" / "#gems succeeds") will fail until
castwide#1225 merges - they exercise
Workspace::Gemspecs#find_gem in an environment with no discoverable
Gemfile, which currently raises Bundler::GemfileNotFound instead of
falling back gracefully. Verified locally that applying castwide#1225's fix
makes both pass with no other changes needed here.
Extracted from castwide#1006 (Improve pin caching) as the
final piece of that PR, stacked on top of the DocMap/Workspace wiring
PR.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The YAML/Psych stdlib resolution spec timed out at 240.33s against a 240s limit in CI - a genuine near-miss (0.14% over), not contention from running multiple PRs' CI concurrently (each job gets its own runner, so concurrent jobs affect queue time, not execution time). Widen that limit and the other 120s limits proportionally to give real headroom against normal run-to-run variance on GitHub Actions' shared runners, rather than re-running and hoping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Earlier, while this PR's PinCache-core piece was still a standalone branch (before combining the engine, wiring, and CLI update into one PR), this spec was pointed at DocMap#cache_all! since that was the only name that existed on master at the time. Now that this PR also includes the DocMap wiring that renames cache_all! to cache_doc_map_gems!, the spec needs to follow that rename too - CI caught the drift (undefined method 'cache_all!' for an instance of Solargraph::DocMap). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bundler.definition raises Bundler::GemfileNotFound (not nil) when no Gemfile is discoverable from the current process's working directory - e.g. when Solargraph is installed and invoked as a standalone gem. The safe-navigation chain in in_this_bundle? doesn't help since the exception happens while evaluating Bundler.definition itself, which crashed find_gem and, downstream, the CLI's unbundled-environment paths exercised by this PR's shell.rb wiring. Same fix as castwide#1225 (open upstream, not yet merged); included directly here so this PR's own CI is green without waiting on that PR to land first. Once castwide#1225 merges, a future rebase of this branch will see it as a no-op. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
apiology
marked this pull request as ready for review
August 2, 2026 22:57
apiology
marked this pull request as draft
August 2, 2026 22:57
Owner
Author
|
Resubmitting this against castwide/solargraph upstream. |
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
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.
Rewrites
PinCachefrom a class-method-based module into an instance-based gem-caching engine, and updates every consumer to match.Why
Fixes a stale-type bug: after adding, updating, or removing an
rbs_collection.yamlentry for a gem, hover/completion/type-checking could keep showing the previous RBS source's results, with no indication anything was stale short of a manualsolargraph cache --rebuild.How
The old cache key conflated a gem's possible RBS sources (bundled types,
rbs_collection, local override, unresolved) into one entry. Replaced with explicitCACHE_KEY_GEM_EXPORT/_UNRESOLVED/_STDLIB/_LOCALclassification plus a content hash of the collection's per-gem lock data.PinCachebecomes an instance scoped to that config, since the key logic now needs to know which config produced it.What's in it
PinCache: instantiable class takingrbs_collection_path/rbs_collection_config_path/directory/yard_plugins;Yardoc,GemPins,RbsMapupdated to match.DocMap,ApiMap,Workspace,Librarydelegate to it; CLIcache/uncache/gemsare thin wrappers.Gemspecs#in_this_bundle?no longer raises with no Gemfile (#1225).An unrelated
RbsMap::Conversionsdead-code fix, in a file this PR already touches, is split out to #41.Testing:
bundle exec rspec— 1595 examples, 0 failures, 64 pending.