[ledger] feat(lsp): Hybrid LSP support for Ruby — split into #1709-#1713 - #1706
[ledger] feat(lsp): Hybrid LSP support for Ruby — split into #1709-#1713#1706benrfairless wants to merge 6 commits into
Conversation
Sorry, something went wrong.
In-process type-aware call resolution for Ruby, mirroring the perl_lsp.c / php_lsp.c shape: - PASS 1 collects classes/modules with lexical nesting, superclasses, include/prepend/extend mixins, and method tables (instance vs singleton classified from the AST; singleton methods key on "<class_qn>.self", which cannot collide with constant paths). - PASS 1.5 infers instance-variable types from @x = Const.new (conflicting assignments latch to no type). - PASS 2 resolves constructors (Const.new -> class node, matching the textual extractor's rewrite), implicit/explicit self dispatch, singleton (class) methods, superclass chains, mixin lookup in Ruby method-resolution order, super, ivar/local/chained receivers. Stdlib seed covers Ruby core classes plus a curated Rails surface; ActiveRecord query methods type as the receiving model (User.find(1) is a User; relation chains keep the model type). Dynamic dispatch (send, method_missing, define_method) and unknown receivers emit no edge (zero-edge guarantee). Only project-defined targets are emitted. Assisted-by: Claude Code:amazon-bedrock/anthropic.claude-fable-5 Signed-off-by: Ben Fairless <ben@oaf.org.au>
Register CBM_LANG_RUBY on the fallback cbm_pxc_run_one path (like Java/Kotlin — no prebuilt tier-2 registry yet). cbm_run_ruby_lsp_cross builds its class table from the project-wide CBMLSPDef[] (constant paths derived by stripping each def's module QN) and registers methods on both instance and singleton receiver keys, since CBMLSPDef does not carry the def self.m split. Ruby joins Rust in bypassing the own+imports module-def filter: constants live in one global namespace and Rails/Zeitwerk autoloads them without require statements, so the filter would starve essentially all cross-file resolution in real Ruby apps. Assisted-by: Claude Code:amazon-bedrock/anthropic.claude-fable-5 Signed-off-by: Ben Fairless <ben@oaf.org.au>
collect_bases_from_field matched neither constant nor scope_resolution children, so a Ruby superclass fell through to the raw-text fallback and base_classes stored "< Base" (operator included) — a name that never resolves. That broke INHERITS resolution and the Ruby LSP's cross-file superclass chain (User < ApplicationRecord < ActiveRecord::Base never reached the ActiveRecord query typing). Same root cause class as the documented Python "(Base)" capture bug. Assisted-by: Claude Code:amazon-bedrock/anthropic.claude-fable-5 Signed-off-by: Ben Fairless <ben@oaf.org.au>
The Ruby constructor-callee rewrite (Widget.new -> "Widget") only fired for bare constant receivers. Scope-resolved receivers (Admin::User.new) kept callee "new", which never resolves and cannot join the LSP's ruby_constructor rows (leaf "new" vs "User"). Treat scope_resolution receivers the same way; the callee carries the full constant path whose bare leaf is what resolution joins on. Assisted-by: Claude Code:amazon-bedrock/anthropic.claude-fable-5 Signed-off-by: Ben Fairless <ben@oaf.org.au>
21 scenarios mirroring test_perl_lsp.c / test_php_lsp.c: the QN contract the resolver's rows depend on, constructor typing and class-node edge targeting, self dispatch, singleton/instance split, inheritance, include/extend mixins, super, ivar typing (plus the conflicting-assignment negative), chained calls, nested modules, ActiveRecord model typing, top-level functions, zero-edge negatives (unknown receiver, send), cross-file def resolution (including an exact repro of the Rails-shaped e2e fixture), and the bare base_classes extractor contract. Assisted-by: Claude Code:amazon-bedrock/anthropic.claude-fable-5 Signed-off-by: Ben Fairless <ben@oaf.org.au>
Badge and language lists move from 10 to 11 Hybrid LSP languages, the Hybrid LSP table gains a Ruby row, and docs/index.html's structured data is corrected to the full current cohort (it still said nine and omitted Perl). The originality guard gains Shopify's ruby-lsp (MIT) as the Ruby reference for defensive copy-detection; the Ruby resolver was authored clean-room from the tree-sitter-ruby grammar and Ruby language semantics (scan verified clean). Resolves DeusData#1701 Assisted-by: Claude Code:amazon-bedrock/anthropic.claude-fable-5 Signed-off-by: Ben Fairless <ben@oaf.org.au>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Sorry, something went wrong.
|
Thank you for the depth of the Ruby work, the clean-room check, and the real-world fixture measurements. The overall direction is relevant, but this is an umbrella contribution rather than one reviewable PR: 2,831 added lines combine two independent extraction fixes, a per-file resolver, a cross-file tier, generated standard-library data, tests, build wiring, and public support claims. Please keep this PR as the ledger and split it into ordered, atomic PRs:
The PRs may be stacked where a later slice needs an earlier one. Keep the originality reference and provenance statement with the resolver slice so the generated data and clean-room boundary can be audited together. This structure is a request for reviewability, not a promise that every slice will merge unchanged. Thank you for offering to split the prerequisite fixes and for making the dependency chain explicit. |
Sorry, something went wrong.
|
Done — split into five ordered PRs, and this one is now the ledger. The table is in the description above.
The originality reference and the provenance statement ship with #1711 as you asked, so the generated data and the clean-room boundary can be audited in one place. The scan output is in that PR's description. On stacking: you said stack where a later slice needs an earlier one, so I checked rather than assumed. I built the resolver on a branch with neither extraction fix and ran its rows — 20/20 pass, because the per-file resolver reads the superclass field off the AST rather than through base_classes, and no per-file row uses A::B.new. So 1, 2 and 3 are independent and only 4→3 and 5→4 are stacked. One caveat on the mechanics: a PR from a fork can only target a branch in your repo, so #1712 and #1713 are based on main and GitHub shows their parents' commits too. I've named the single commit that belongs to each in their descriptions. Two deviations from your outline, both flagged in the ledger. cbm_run_ruby_lsp_cross moved from slice 3 to slice 4, because in the original commit nothing called it until the pipeline wiring landed — moving it means each slice's tests cover everything that slice adds, and the two halves reassemble byte-identically. And docs/index.html is dropped rather than partially corrected: it carries a pre-existing error on the exact strings the Ruby edit touches, so I'd either be writing something I know is wrong or folding an unrelated fix into a feature PR. Splitting also caught two things I'd rather surface than quietly fix. The A::B.new rationale in this PR was wrong — I said the callee stayed "new"; it actually kept the whole Admin::User.new expression, which the reproduce-first test shows. And the README row advertised prepend, method_missing and define_method with no tests behind them, so I added two rows to #1711 rather than soften the wording. Your point about umbrella PRs, demonstrated. Last thing, unrelated to this series but relevant to reading the CI: main is red on cli_zero_argument_tool_never_reads_stdin_issue1359 and cli_stdin_args_gate_tracks_tool_schema_issue1359 from the #1181 × #1359 composition, so all five will show a red ci-ok until #1704 merges. I verified the failures are pre-existing by running the same suite on unmodified origin/main and getting an identical 249 passed / 32 failed. Thanks for the review — the structure is a lot easier to defend slice by slice. |
Sorry, something went wrong.
The five slices
Ordering follows the review exactly. Stacking does not: the review said stack
where a later slice needs an earlier one, so only 4→3 and 5→4 are stacked.
Measured, not assumed. I built slice 3 on a branch containing neither
extraction fix and ran its rows: 20/20 pass. The per-file resolver reads the
superclass field straight off the AST rather than through base_classes, so
it does not need slice 1, and no per-file row uses A::B.new, so it does not
need slice 2. Slices 1, 2 and 3 are therefore independently mergeable in any
order.
Deviations from the outline, and why
1. cbm_run_ruby_lsp_cross moved from slice 3 to slice 4. In the original
commit it shipped with the resolver, where nothing called it until the pipeline
wiring landed — ~154 lines of untested-in-its-own-PR code. Moving it makes each
slice's tests exercise everything that slice adds. The two halves reassemble
byte-identically to the original file; I checked with git diff against
this branch.
2. docs/index.html dropped entirely. The original commit corrected a
pre-existing error there (structured data said nine language families and
omitted Perl, both wrong before this work). Those are the same strings the Ruby
edit lands on, so adding Ruby without fixing them would mean authoring a
sentence I know is wrong, and fixing them in slice 5 would mix an unrelated
correction into a feature PR. Left for its own change.
Corrections found while splitting
Splitting surfaced two things worth stating plainly rather than quietly fixing:
stayed "new". Writing the reproduce-first test showed the real pre-fix
callee is the whole Admin::User.new expression. The fix is unchanged and
still needed; fix(extraction): reduce A::B.new callee to the constant #1710 states the actual behaviour.
and define_method were named in the README row. All are genuinely
implemented, but rather than weaken the docs I added rubylsp_prepend_mixin
and rubylsp_dynamic_dispatch_family_no_edge to feat(lsp): add the Ruby per-file resolver #1711 so every claim in the
table is backed.
Both are exactly the kind of thing an umbrella PR hides, which rather makes the
reviewer's point.
Verification
Full canonical leg (scripts/test.sh, all contract steps 0a–0s, clean
sanitizer build, all 140 suites) on the five slices merged together:
All 32 failures are in cli and reproduce identically on unmodified
origin/main — every failing site in my run also fails on a clean baseline,
and the cli totals match exactly (249 passed / 32 failed on both). Of those:
install/uninstall tests. Local-only; CI's test-diag on this branch showed
7573 passed / 2 failed.
make -f Makefile.cbm lint-ci passes. check-lsp-originality.sh --lang ruby
reports CLEAN.
One local-environment note for anyone reproducing: tests/test_vm_worktree_manifest.sh
fails on a machine whose global gitignore excludes target/, because its
fixture does git add target/tracked.txt. Not a repo bug — CI has no global
ignore file — but it needs
GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.excludesFile GIT_CONFIG_VALUE_0=/dev/null
to get through Step 0b locally.
Also noticed, not changed
SUITE(extraction_inheritance) marks inherit_python, inherit_typescript,
inherit_tsx, inherit_php and inherit_kotlin as RED: (broken
extractors). All five pass today, so those markers are stale. Left alone to
keep #1709 to one thing.
Refs #1701
Assisted by OpenCode using anthropic/claude-opus-5.