Conversation
Building YARD documentation for the parser gem takes over a minute on a cold cache, while its RBS collection types already describe the API. Caching parser measured 73.13s with YARD and 0.08s without. PinCache.suppress_yard_cache? reports true for a gem named in YARD_SUPPRESSED_GEMS whose RBS types resolved. Every place that decides to build YARD pins now consults it: DocMap#cache, Shell#cache, the named-gem path in Shell#gems, and Shell#do_cache. DocMap#deserialize_combined_pin_cache treats a suppressed gem as having an empty YARD half rather than an uncached one, so combined pins still get written and the gem stops reading as uncached on every load. The trade-off is coverage: parser yields 9,175 combined pins with YARD and 178 from RBS alone. Much of that gap is likely private and generated node classes, but that has not been verified against what users complete on.
undercover reported the method as 0.0% covered on this branch. The existing #cache and #gems examples reach it only through a subprocess shim, so none of its lines record coverage. The new examples call the private method directly with a stubbed Workspace, RbsMap and PinCache, and let the real PinCache.suppress_yard_cache? decide: a suppressed gem whose RBS collection resolves skips the YARD build, an unresolved cache key does not, and a gem outside the suppression list does not.
The existing specs for this branch all reach through PinCache, GemPins and RbsMap.from_gemspec, so they assert how the current caching layer is built rather than what a user gets. castwide#1369 deletes all three, which would leave the behaviour with no coverage at all. Yardoc.load! is the one step on the YARD path that both the DocMap and the Collection implementations reach unconditionally, so asserting on it holds whichever one is caching the gem. Rebuild is forced so the decision under test is the suppression rather than whatever the gem has cached already; Thor reads the option by symbol, and a plain string-keyed Hash reads back as nil, which silently made an earlier draft of these pass on cache state instead. Verified in both directions: three pass on this branch, and with doc_map.rb, pin_cache.rb and shell.rb restored from 6dcb733 exactly one fails, the parser-skips-YARD example. The other two are a control and a guard against a suppression that also drops the RBS types, and pass either way by design.
The eleven specs added by this branch all reached through PinCache, GemPins, DocMap or RbsMap.from_gemspec. castwide#1369 deletes every one of those, so the specs assert how today's caching layer is built rather than what a user gets, and would leave the behaviour uncovered the moment that lands. Shell#cache and Shell#gems are public commands in both designs, and Yardoc.load! is the one step on the YARD path that the DocMap and the Collection implementations both reach unconditionally. Five examples on those seams carry the same intent: the gem is not documented by YARD, its RBS types still resolve, and a gem outside the list is unaffected, each checked through both commands. Removed in exchange: spec/pin_cache_spec.rb, the doc_map_spec context, and the shell_spec do_cache block. Two assertions have no public-seam equivalent and are simply gone. A listed gem whose RBS fails to resolve cannot be set up without stubbing RbsMap.from_gemspec, which is one of the removed APIs. Shell#do_cache is private, has no counterpart in 1369, and its caller iterates every installed gemspec, so the named-gem path in Shell#gems stands in for it. Verified in both directions: 39 pass here, and with doc_map.rb, pin_cache.rb and shell.rb restored from 6dcb733 exactly two fail, the two that assert YARD is skipped.
Every line this branch changed lived in doc_map.rb, pin_cache.rb or the part of shell.rb that drove them, and castwide#1369 deleted the first two and rewrote the third. There is nothing here to reconcile line by line, so the merge takes master whole and the behaviour comes back in the next commit, written against Collection::Gem. The five specs this branch added to shell_spec.rb go with it; they return as spec/suppress_yard_spec.rb, which reaches the same Yardoc.load! through the commands rather than through a DocMap.
Opening a workspace runs yardoc over every cacheable gem it requires, and for `parser` that is over a minute of work whose pins an RBS collection already carries. The five specs ported from castwide#1339 measure it: before this change the two parser examples ran 15.3 and 13.5 seconds against 5.9 and 5.6 for the backport controls, and the whole file took 85 seconds. It now takes 5. Collection::Gem skips Yardoc.load! for a gem on its list when the workspace collection carries that gem, and returns the gem RBS pins alone. The collection pins arrive as they already did, workspace-wide, from External#load_rbs_collection. A gem read without its YARD documentation holds different pins from one read with it, and Metagem#cache_name is a bare basename shared by every workspace on the machine, so the two variants get separate entries and uncache clears both. Every caller that loads or asks after a gem collection now says which variant it means: External#process_gem, External#cache_changed?, ApiMap.load_with_cache, and the three Shell commands. The collection reading moves out of External into RbsCollection, because Shell needs the same answer and building an External to get it would parse every collection signature first. External keeps its two accessors and delegates. Base.uncache now goes through an instance method so Gem can clear both of its entries without duplicating the removal. Verified against master: the five specs fail there, two on `Yardoc.load!` having been received. Full suite 1619 examples, 0 failures, 78 pending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E8ckZEYYMDXe9eDNnyxaim
The only conflict was shell.rb. castwide#1373 folded the cache command into gems and renamed the result to cache, leaving gems a Thor alias, so both sides had rewritten the same two methods. Master's structure is taken whole, with this branch contribution put back on top of it: Collection::Gem.load receives rbs_collection:, so a gem whose RBS collection data suffices still skips the expensive YARD generation this branch exists to avoid. The standalone cache GEM command is dropped, because master consolidated command covers it. Both sides had written a @PARAM tag for the renamed parameter; the surviving one names gem_names.
castwide#1373 left gems a Thor CLI alias rather than a method, so the two examples calling it on an instance raised NoMethodError once this branch followed master structure. Both existed to cover gems as a second way in, and both assert exactly what the cache example beside them already does -- one that YARD is not read for a gem on the list, the other that it is read for a gem outside it. With the second entry point gone there is nothing left for them to cover, so retargeting them at cache would only duplicate coverage.
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.
This PR was written by Claude Code on behalf of @apiology.
Carried on the fork after castwide#1339 was closed as not planned, in favour of castwide#1378. The specs below are the part worth keeping alive regardless of which implementation wins.
Problem:
solargraph cache parsershells out toyardocacross the gem's whole source tree even when the RBS collection already declares the types Solargraph reads from it.Solution: ask the RBS collection whether it covers a gem before caching it, and skip
Yardoc.load!when it does.spec/suppress_yard_spec.rbpins the behaviour from outside the implementation:Yardoc.load!is not reached for a covered gem, the RBS-declared type onParser::AST::Node#childrenstill resolves, and a gem outside the collection still gets its yardoc. All three pass against castwide master at 2b9e317.