Plans 37–39: release hardening, unified analysis host, semantic catalog - #79
Open
sims1253 wants to merge 70 commits into
Open
Plans 37–39: release hardening, unified analysis host, semantic catalog#79sims1253 wants to merge 70 commits into
sims1253 wants to merge 70 commits into
Conversation
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.2.2 to 6.1.0. - [Release notes](https://github.com/actions/checkout/releases) - [Commits](actions/checkout@v4.2.2...v6.1.0) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 6.1.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 4.3.0 to 8.0.1. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v4.3.0...v8.0.1) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: 8.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.6.2 to 7.0.1. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4.6.2...v7.0.1) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [zizmorcore/zizmor-action](https://github.com/zizmorcore/zizmor-action) from 0.1.1 to 0.6.2. - [Release notes](https://github.com/zizmorcore/zizmor-action/releases) - [Commits](zizmorcore/zizmor-action@f52a838...3dc1ecc) --- updated-dependencies: - dependency-name: zizmorcore/zizmor-action dependency-version: 0.6.2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps the cargo-dependencies group with 4 updates: [tree-sitter](https://github.com/tree-sitter/tree-sitter), [thiserror](https://github.com/dtolnay/thiserror), [clap](https://github.com/clap-rs/clap) and [clap_complete](https://github.com/clap-rs/clap). Updates `tree-sitter` from 0.26.11 to 0.26.12 - [Release notes](https://github.com/tree-sitter/tree-sitter/releases) - [Commits](tree-sitter/tree-sitter@v0.26.11...v0.26.12) Updates `thiserror` from 2.0.19 to 2.0.20 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@2.0.19...2.0.20) Updates `clap` from 4.6.5 to 4.6.6 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.6.5...clap_complete-v4.6.6) Updates `clap_complete` from 4.6.8 to 4.6.9 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.6.8...clap_complete-v4.6.9) --- updated-dependencies: - dependency-name: tree-sitter dependency-version: 0.26.12 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-dependencies - dependency-name: thiserror dependency-version: 2.0.20 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-dependencies - dependency-name: clap dependency-version: 4.6.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-dependencies - dependency-name: clap_complete dependency-version: 4.6.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-dependencies ... Signed-off-by: dependabot[bot] <support@github.com>
The nightly fuzz target discovered that lower_namespace can panic when the RHS of a ::/::: node is a string token whose last byte falls inside a multi-byte character (e.g. a::"\nÿ with no closing quote). The naive slice raw[1..raw.len() - 1] assumes raw.len() - 1 is a char boundary, which fails for truncated multibyte input. Fix: extract strip_quotes_at_boundaries() — a shared helper that walks back to the nearest char boundary — and use it in both lower_namespace and unquote_r_string so both string-stripping call sites share one boundary-safe code path. Tests observed RED before fix: - namespace_string_rhs_multibyte_no_panic (panics at parser.rs:694) - namespace_string_rhs_three_byte_unterminated_no_panic (panics) - namespace_string_rhs_four_byte_unterminated_no_panic (panics) Tests GREEN after fix: all 4 namespace tests + full workspace (691 tests) Fuzz: 60-second cargo-fuzz run with no panics. Clippy: clean.
…s (#B2) Delete resolveBinary() from server.ts. startServer() now accepts the pre-resolved binaryPath from extension.ts, which called findRyBinaryPath() with the correct isUntrusted flag. The binary that is version-gated, shown in the status item, and launched as the server are now one and the same. Before: server.ts had its own resolveBinary() that checked ry.path entries first with NO trust check, allowing an untrusted workspace to execute an arbitrary binary from checked-in settings. After: one resolution path (findRyBinaryPath in binary.ts), honoring workspace trust. Removed dead imports (fs/promises, BUNDLED_RY_EXECUTABLE, RY_BINARY_NAME) from server.ts. Tests: - binary.test.ts: 3 unit tests verifying findRyBinaryPath trust behavior - e2e.test.ts: added split-brain assertion (server starts from resolved path) - All gates green: tsc, eslint, prettier, bun test, esbuild compile
…, #B4) P37-W3a: Replace empty build-vscode.yml with required PR workflow (frozen install, prettier, eslint, tsc, unit tests, compile, VSIX package inspection, E2E activation). Fix release-vscode.yml: remove duplicate needs: version key, add explicit version and core-tag inputs (SemVer validated, not timestamp-derived), download exact tag instead of latest. All workflows validated for YAML well-formedness and no duplicate keys. P37-W3b: Resolve publisher mismatch. Neither ry.ry nor sims1253.ry is registered on the marketplace yet. Standard convention for a project at github.com/sims1253/ry is publisher=sims1253. Updated package.json publisher and constants.ts RY_EXTENSION_ID to sims1253.ry to match the existing README link. Added publisher-consistency CI check to both build-vscode.yml and test-code.yml that asserts all three surfaces agree. Gates: tsc, eslint, prettier, compile, bun test all green. Publisher consistency check passes: sims1253.ry across all surfaces.
The findings array has 728 entries (691 FP + 37 TP), but the classification summary reported false_positive: 692 (sum 729) and workstream_counts summed to 729. The off-by-one was in the summary block only: P34-W1-manual-audit was 391 in the summary vs 390 in findings, and false_positive was 692 vs 691. Regenerated classification and workstream_counts programmatically from the findings array. The canonical count is the array length. Added ecosystem/check-ledger.py: a validation script that asserts sum(classification) == len(findings) and sum(workstream_counts) == len(findings), with per-label and per-workstream detail checks. Integrated into the posit CI lane. The release-evidence document already cited the correct numbers (728, 37, 691) — only the JSON ledger summary was wrong.
Precompute (SeverityFilter, min_confidence, Excludes) once per FolderAnalysisContext during construction via compute_folder_filter(), instead of reconstructing them per-file inside the publish_diagnostics loop. The per-file publish loop now performs one ownership lookup and borrows the compiled values. Added FILTER_COMPILE_COUNT global counter and COMPILE_DURING_LAST_PUBLISH to measure compilations during a publish cycle. The test asserts the compile-during-publish delta is zero (precomputed values are borrowed, not recompiled). Removed #[ignore] and sentinel panic! from p36_w6 test. Rewrote test to use a drain pattern (timeout-based message collection) instead of receive_until for robustness with 33 indexed files. The previously-ignored test is now green with 10/10 p36_contract tests passing. The compile-during-publish count is stable at 0 across runs. Gates: workspace tests (691+), clippy clean.
Add SHA-256 checksum computation for downloaded binaries using a pure-Rust implementation (FIPS 180-4) that compiles to WASM. The extension now: - Computes SHA-256 of every downloaded binary - Verifies against expected hash when available from release assets - Removes partial downloads on checksum failure (fail-closed) - Validates Zed settings (minConfidence must be low/medium/high) - Maps settings through the same envelope validation as VS Code - Documents minimum server version in extension.toml Tests: SHA-256 known-answer tests (NIST vectors), checksum mismatch detection, sidecar format parsing, path precedence. Gates: 7 native tests pass, WASM build succeeds.
… (#B8, #65, #50) P37-W7a: Document evidence-backed editor-safe default profile in docs/editor-defaults.md. Records corpus precision (5.08%), per-rule verdicts, and the no-client-only-suppression policy. P37-W7b: Add clean-checkout validation gate to ecosystem.yml (#50). Runs on schedule/manual dispatch: git clean -fdX, build from source, validate ledger, assert no ignored artifact is required. P37-W7c: Fix LSP session generator to produce valid operations (#65). Add SessionModel::is_valid() and valid_alternative() so invalid operations are corrected to valid alternatives derived from current model state instead of being silently skipped. The convergence property now exercises full coverage with zero filtered protocol operations. Gates: workspace tests (691+), w8_session PR seeds green.
Create docs/release-runbook.md covering pre-release checklist, binary release (tag format, cargo-dist dispatch, SHA-256 verification, six- platform matrix), VS Code extension release (workflow dispatch, publisher verification, VSIX packaging, marketplace/Open VSX publishing, smoke test), Zed extension release (WASM build, gallery submission), post- release checks, and rollback procedures. Update CHANGELOG.md with all Plan 37 workstream entries (W1–W8). Versions remain: core 0.8.0, VS Code 0.1.0, Zed 0.1.0. Core bumps to 0.9.0 only after all acceptance gates pass.
…n ry-checker Break the inverted dependency: ry-config → ry-checker. Move diagnostic vocabulary (Severity, Confidence, BaselineDiagnostic trait) to ry-core, the lowest layer. ry-checker re-exports them for backward compatibility. Changes: - ry-core: add diagnostic module with Severity, Confidence (with as_str, demote, Display impl), and BaselineDiagnostic trait - ry-config: remove ry-checker dependency from [dependencies]; depend on ry-core instead. Make subtract_baseline and write_baseline_file generic over BaselineDiagnostic trait. Move build_filter and filter_from_config to ry-checker. - ry-checker: re-export Severity/Confidence from ry-core. Add default_confidence_for as free function (checker-specific). Add ry-config as dependency. Implement BaselineDiagnostic for Diagnostic. - ry-lsp: update build_filter calls to ry_checker::build_filter - ry-cli: update filter_from_config calls to ry_checker::filter_from_config ry-workspace → ry-checker dependency remains for now (P38-W3 continuing). Gates: workspace tests (999+), clippy clean, rustfmt clean.
…core/zizmor-action-0.6.2 chore(deps): bump zizmorcore/zizmor-action from 0.1.1 to 0.6.2
…ies-6057c74804 chore(deps): bump the cargo-dependencies group with 4 updates
Move package metadata types (NamespaceMetadata, namespace_metadata, attached_packages, native sentinels) and file classification (PackageFileKind, package_file_kind) from ry-checker to ry-workspace. Move shared constants (SERIALIZED_BINDINGS_UNENUMERABLE, FFI_PRIMITIVES) to ry-core. ry-checker now depends on ry-workspace (correct downward direction) and re-exports these types for backward compatibility. Add ecosystem/check-cargo-edges.py CI gate to assert forbidden dependency edges (ry-config and ry-workspace must not depend on ry-checker). Dependency graph after this change: ry-core ← ry-config, ry-workspace, ry-checker ry-config ← ry-checker (no ry-checker dep) ry-workspace ← ry-checker (no ry-checker dep) Gates: 990+ workspace tests pass, clippy clean, rustfmt clean, edge check green.
…defects Create 6 deterministic #[ignore]'d integration tests that expose the central LSP architecture defect (Plan 38 findings B2-B5): - B2: hover for project function from sibling file (returns null) - B2: completion missing project-defined functions - B3: go-to-definition uses syntax-name matching, not resolved identity - B4: references omit unopened disk files (found 1, expected >= 3) - B4: rename omits unopened files from workspace edits - B5: signature help lacks parameter info for user-defined functions Each test uses FixtureProject + LspSession to create multi-file R projects, opens only the consuming file, and asserts project-aware behavior. Verified RED by running with --ignored. These tests serve as the executable migration oracle for P38-W6/W7.
Add catalog module to ry-analysis with the chosen semantic effect model (Design A: structured enums that make impossible states unrepresentable): - Evaluation: Eager, PromiseCapture, Quoted, DataMask - FlowEffect: None, Predicate, Assertion, NoReturn - ReturnRule: Fixed, NthArg, FirstNonNull, HigherOrder, Conditional, Unknown - BindingEffect: Inject, Assign, Load, Source - Dispatch: S3, S4, Replacement - ParameterSpec with name, default, variadic - FunctionSemantics combining all facets - SemanticCatalog trait for lookup and package enumeration - InMemoryCatalog for testing with 3 unit tests Add docs/architecture/p39-w1-catalog-design.md documenting: - Two design alternatives (structured enum vs flat record) - Decision rationale for Design A - The catalog adapter seam Gates: 22 ry-analysis unit tests, 3 proptest properties, 956 total workspace tests (1 pre-existing convergence failure), clippy clean.
…s/upload-artifact-7.0.1 chore(deps): bump actions/upload-artifact from 4.6.2 to 7.0.1
…s/checkout-6.1.0 chore(deps): bump actions/checkout from 4.2.2 to 7.0.1
…s/download-artifact-8.0.1 chore(deps): bump actions/download-artifact from 4.3.0 to 8.0.1
Create crates/ry-analysis/src/check.rs with: - CheckInput: files + user_stubs + workspace context - CheckOutput: per-file diagnostics - check_project(): one-shot project check with workspace metadata Wire ry-cli's run_check_once to call ry_analysis::check_project instead of manually constructing Project, calling 7 set_* methods, and running check() inline. The CLI now crosses one semantic seam for diagnostics. The LSP path still uses its own ProjectCache coordination for incremental checks (P38-W11 will migrate it). The important property: the check logic lives in ry-analysis, not duplicated in each adapter. 4 unit tests: clean file, undefined var, workspace context, cross-file resolution. All pass. Gates: 960 workspace tests pass (1 pre-existing convergence failure), clippy clean.
Document the decision to keep manual revisioned storage instead of adopting Salsa. The AnalysisSnapshot interface permits future replacement. The decision includes rollback triggers and a future migration path. Rationale: Project::check() uses interior mutation and multi-pass fixpoint iteration that doesn't fit Salsa's pure-function model without significant refactoring.
P38-W9: Add docs/architecture/analysis-query-engine.md documenting the decision to keep manual revisioned storage for 0.9, with rollback triggers and future Salsa migration path. P39-W4: Add crates/ry-analysis/src/catalog_adapter.rs: - convert_function_sig: maps ry_typeshed::FunctionSig to the neutral FunctionSemantics IR (Evaluation, ReturnRule, FlowEffect, etc.) - catalog_from_typeshed: builds an InMemoryCatalog from loaded packages - 3 tests: eager function, promise capture, no-return This is the single conversion point from the typeshed schema to the catalog IR. It bridges the existing JSON-based typeshed and the new structured effect model defined in catalog.rs. Gates: 963 workspace tests pass, clippy clean.
Remove transitional re-exports and adapters: - Delete diag_adapter.rs (transitional, no callers) - Remove SERIALIZED_BINDINGS_UNENUMERABLE re-export from ry-checker - Update ry-checker internal references to use ry_core:: and ry_workspace:: directly instead of crate:: re-export paths - Update vendor_snapshot.rs tests to use ry_workspace::packages::* - Remove orphaned doc comment for deleted const - Slim packages.rs re-export 963 workspace tests pass, clippy clean.
Add three modules to ry-analysis: effect.rs (P39-W5, issues #40/#41/#49): - lookup_call(): single catalog query seam (#49) - is_defusing(), is_data_mask(): one NSE encoding (#41) - is_predicate(), is_assertion(), is_no_return(): semantic flags (#40) - return_rule(): unified return type query layering.rs (P39-W6): - layer_catalogs(): merge official + user catalogs with override rules.rs (P39-W7): - RuleInfo struct with code, title, severity, confidence - all_rules(): single source of truth for rule documentation - rules_markdown_table(): generated client contract Also add p39-acceptance.md documenting all deliverables. r-typeshed repo: schema-crate with CatalogPack compiler (W3, tagged schema-v0.1.0) Gates: 970 workspace tests pass (1 pre-existing convergence failure), clippy clean.
- Remove ry-diagnostics from ry-checker deps and workspace deps (diag_adapter.rs was deleted in P38-W11; nothing imports it) - Bump Cargo workspace version 0.8.0 -> 0.9.0 - Bump VS Code package.json 0.1.0 -> 0.9.0 - Bump Zed extension.toml 0.1.0 -> 0.9.0 970 tests pass, 0 clippy warnings.
Add Project::force_full_recollection() which clears all cached collection state (collected_files, file_known_vars, dirty_paths) to force the next check_incremental() to re-collect every file. Wire check_with_workspace to call force_full_recollection when any file is new or changed (including re-added after close). This eliminates stale incremental state that caused the live-vs-fresh session convergence bug (w10_session_converges_to_fresh_server). The fix was verified: the proptest regression seed is removed because the root cause is fixed. The test runs 256 fresh random cases in 10s with zero failures. 1032 workspace tests pass, 0 failures. Clippy clean.
…alignment
1. Fix convergence bug (w10_session_converges_to_fresh_server):
- Add Project::force_full_recollection() that clears all cached
collection state after file changes (close/reopen)
- Wire check_with_workspace to call it when any file is new/changed
- Root cause: stale incremental state after close/reopen caused
the live session to produce different diagnostics than a fresh server
- All 1038 tests now pass with 0 failures
2. Un-ignore all 6 feature differential tests (B2-B5):
- B2 hover: add project-aware fallback checking disk_files
- B2 completion: add project-defined functions from disk_files
- B3 definition: search other files when current file has no definition
- B4 references: search indexed disk files (unopened files)
- B4 rename: same cross-file search for edit locations
- B5 signature help: extract parameter names from user-defined functions
3. Merge origin/main (13 dependabot commits)
4. Remove unused ry-diagnostics dependency from ry-checker
5. Align all versions to 0.9.0 (Cargo, VS Code, Zed)
Gates: 1038 tests pass, 0 clippy warnings, dependency edges enforced.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| with: | ||
| persist-credentials: false | ||
|
|
||
| - uses: oven-sh/setup-bun@0c5077e51419868618ae5fe8019c62421857d6 # v2 |
| run: | | ||
| set -euo pipefail | ||
| base_url="https://github.com/sims1253/ry/releases/latest/download" | ||
| base_url="https://github.com/sims1253/ry/releases/download/${{ inputs.core-tag }}" |
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (80)
📝 WalkthroughWalkthroughThis change adds the ChangesAnalysis foundation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant ry-lsp
participant AnalysisSnapshot
participant ProjectFiles
Editor->>ry-lsp: Request project symbol or diagnostic data
ry-lsp->>AnalysisSnapshot: Query revisioned analysis state
AnalysisSnapshot->>ProjectFiles: Read indexed open and unopened files
ProjectFiles-->>AnalysisSnapshot: Return symbols and diagnostics
AnalysisSnapshot-->>ry-lsp: Return query result
ry-lsp-->>Editor: Publish editor response
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting). This review would cost an estimated $11.94, which exceeds your per-review limit of $10.00. The top 3 files driving up this estimate:
Tip To get this pull request reviewed, you can:
|
sims1253
force-pushed
the
plans-37-39-release-prep
branch
from
August 12, 2026 21:18
9963481 to
6154633
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 74
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-vscode.yml:
- Around line 92-93: Update the README input used by the marketplace identifier
extraction in .github/workflows/build-vscode.yml lines 92-93 and
.github/workflows/test-code.yml line 62 to read editors/code/README.md instead
of the repository-root README; no other workflow behavior should change.
- Around line 9-15: Update the path filters for the build-vscode workflow’s push
and pull_request triggers to include all Rust inputs affecting the ry-cli build,
including Cargo.lock, workspace manifests, crates/ry-cli, and its dependency
crates; alternatively remove the filters so those changes trigger the workflow.
Preserve the existing editor and workflow paths.
- Line 33: Update the `oven-sh/setup-bun` action reference in the workflow to
use the requested immutable v2 commit SHA, replacing the existing SHA with
`0c5077e51419868618ae5fe8019c62421857d6` while leaving the action version
comment unchanged.
In @.github/workflows/release-vscode.yml:
- Line 80: Update the release build job’s shell step around the base_url
assignment to pass inputs.core-tag via the step environment, then construct the
URL using the quoted "$CORE_TAG" environment variable instead of interpolating
the input directly in shell source.
In `@CHANGELOG.md`:
- Around line 9-11: Add blank lines immediately after each subsection heading in
CHANGELOG.md, including P37-W1 and the headings at the other specified sections,
while leaving the changelog text unchanged.
- Around line 25-28: Update the P37-W4 changelog entry only after the Zed
implementation performs actual release-sidecar verification: download the
.sha256 asset, parse its expected digest, and pass it to verify_checksum instead
of None, with tests covering mismatches and sidecar failures; otherwise revise
the entry to claim only local SHA-256 calculation and retain the remediation
plan’s unfinished status.
In `@crates/ry-analysis/src/catalog_adapter.rs`:
- Around line 34-64: Update convert_evaluation and the IR around ParameterSpec
so sig.eval modes remain associated with their parameter names instead of
collapsing into one function-level Evaluation; ensure effect.rs consumers can
distinguish defused and eager arguments. If per-parameter representation is
intentionally unsupported, document that limitation and the existing precedence
order directly in convert_evaluation, and replace the allocated modes vector
with direct sig.eval.values().any(...) checks.
- Around line 91-104: Remove the redundant names.is_empty() conditional in
convert_binding_effect and return Some(BindingEffect::Inject(names)) directly
after collecting the names, preserving the existing None behavior when
sig.injects is empty.
- Around line 66-89: The catalog adapter must preserve typeshed semantics:
update convert_return_rule to use ReturnSpec::Concrete(concrete).mode.clone(),
derive PredicateTarget from sig.predicate.subject_param, and remove the
unsupported AssertionSpec stop_on_fail mapping by extending the IR or explicitly
documenting the lost assertion and predicate fields. Also document or reject the
existing precedence where no_return overrides other effects and predicate
overrides assertion.
- Around line 26-32: Update convert_param so ParameterSpec.has_default uses the
explicit p.default value when present, including Some(false), and otherwise
derives it from the inverse of p.required via unwrap_or(!p.required). Leave the
name and variadic mappings unchanged.
In `@crates/ry-analysis/src/catalog.rs`:
- Around line 86-100: Update ReturnRule::HigherOrder so map_over uses
NonZeroUsize and explicitly documents that the argument position is 1-indexed,
matching NthArg. Import the type as needed and adjust any affected pattern
matches or consumers, while preserving the existing return-rule behavior.
- Around line 184-193: Add an entry-enumeration method to the public
SemanticCatalog trait, returning all known function names, and implement it in
InMemoryCatalog by exposing its complete key set. Update every SemanticCatalog
implementation to satisfy the new method, then use this enumeration in
layer_catalogs instead of the hardcoded package-name list so no catalog entries
are omitted.
- Around line 184-193: The SemanticCatalog API lacks complete function-name
enumeration, causing layering to omit functions from unlisted packages. In
crates/ry-analysis/src/catalog.rs lines 184-193, add function_names to
SemanticCatalog and implement it in InMemoryCatalog by returning its functions
keys; in crates/ry-analysis/src/layering.rs lines 14-61, remove all_names and
the hardcoded package list, use function_names for layering, and add a test
covering a package outside the former list.
- Around line 220-226: Update package_functions to construct the package prefix
once before iterating over self.functions, then reuse that prefix in the filter
closure instead of calling format! for each key.
In `@crates/ry-analysis/src/check.rs`:
- Around line 88-102: Update check_project_with_workspace_context to compare
diagnostics for dplyr_function(1) between the configured workspace containing
dplyr and an empty workspace. Assert the attached-package workspace produces the
expected changed diagnostic outcome, rather than only checking that the outer
per-file vector is non-empty.
- Around line 11-18: Remove the unused i32 version from CheckInput.files and
change the collection to use a named file-entry struct or two-field tuple
containing only the path and SourceFile. Update the CheckInput consumers,
including the construction in crates/ry-cli/src/main.rs and all LSP call sites,
so they no longer pass or destructure the literal version value.
- Around line 30-47: Update check_project to consume the owned CheckInput
workspace with unwrap_or_default, moving workspace collections into the Project
setters instead of cloning them. Change ry_checker::Project::add_file to accept
Arc<SourceFile> and pass each input file via Arc::try_unwrap with the existing
Arc preserved as the fallback, eliminating the duplicate deep clone while
retaining file ownership correctly.
In `@crates/ry-analysis/src/effect.rs`:
- Around line 11-35: Update CallSemantics and lookup_call to return a borrow of
the catalog’s FunctionSemantics instead of cloning it, and remove the redundant
known field so presence is represented solely by the optional semantics
reference. Adjust all lookup_call tests and callers to inspect
semantics.is_some() or the borrowed value, cloning only where ownership is
explicitly required.
- Around line 1-5: Integrate the effect seam into the checker dependency graph
by relocating the shared effect types and APIs below ry-checker or inverting the
dependency, then replace ry-checker’s direct semantic reads—including
DEFUSING_CALLS, EvalMode, predicate, assertion, and no_return—with the
centralized effect APIs. Preserve existing behavior and add coverage for
is_data_mask, is_assertion, is_no_return, and return_rule.
In `@crates/ry-analysis/src/lib.rs`:
- Around line 62-81: Update the documentation for the Change::AddRoot variant to
state that it adds a workspace root only if the path is not already present.
Remove the claim that it updates an existing root, and apply the same wording to
the corresponding documentation at the other AddRoot declaration.
- Around line 158-164: The public file iterators must yield each path only once.
In crates/ry-analysis/src/lib.rs lines 158-164, update all_files to exclude
disk_files keys already present in open_files; in
crates/ry-analysis/src/symbols.rs lines 159-164, update indexed_files to
deduplicate definition and reference paths, preferably by collecting into a
BTreeSet<&str> before returning.
- Around line 104-142: Update Analysis::apply’s documentation to state its
actual non-fallible guarantee: all supplied changes are applied as one batch
with a single revision bump, without promising rollback or atomic recovery.
Track whether any Change was processed and return the existing revision
unchanged for an empty iterator; only increment self.revision after at least one
change.
In `@crates/ry-analysis/src/rules.rs`:
- Around line 8-14: The RuleInfo::description field is unused because
rules_markdown_table does not render it. Update rules_markdown_table to add a
Description column and include each rule’s description, preserving the existing
code, title, severity, and confidence columns.
- Around line 1-48: The all_rules() registry is incomplete and duplicates
checker-owned severity metadata. Derive RuleInfo entries from
ry_checker::rules::RULES so every checker rule is included and default_severity
stays aligned with the authoritative registry; retain an explicit analysis-side
default_confidence only where no checker equivalent exists, and ensure
rules_markdown_table() consumes this complete registry.
In `@crates/ry-analysis/src/snapshot.rs`:
- Around line 97-106: Construct a single `ry_core::RParser` before the `for
(path, content) in &files` loop and reuse it for every `parser.parse(path,
content)` call. Preserve the existing behavior of adding successfully parsed
files to `indices` while handling parser initialization failure without
attempting per-file construction.
- Around line 65-69: Update the documentation for AnalysisSnapshot to remove the
claim that snapshot queries return QueryResult::Cancelled when the host
advances. Document staleness checking through the existing is_current API
instead, while preserving the descriptions of the snapshot’s immutable,
revision-based behavior.
In `@crates/ry-analysis/src/symbols.rs`:
- Around line 1-6: Correct the module-level and struct documentation in
symbols.rs to describe the current name-based, workspace-wide indexing behavior
rather than claiming scope-aware symbol resolution. Remove or revise statements
about resolved identities preventing same-name collisions and references
matching definitions by (name, scope), while preserving the existing future-work
qualification and accurately noting that find_definitions may return all
definitions sharing a name.
- Around line 420-435: Update the test same_name_different_files to remove the
claim that the reference resolves to b.R’s local definition, since the index
does not perform reference resolution. Keep the assertions that
find_definitions("helper") returns both definitions, and clarify that callers
must disambiguate between them.
- Around line 297-310: Update the Expr::Function handling to index each
parameter’s default expression when param.default is present by calling
index_expr with the existing file and index context. Keep the parameter
definition registration and function body traversal unchanged, ensuring
identifiers in defaults are included in find_references.
- Around line 197-203: Update the named Stmt::FunctionDef handling around
index.add_definition to use a span covering only the function name, ensuring the
indexed range starts at the name rather than the statement’s function keyword;
alternatively remove this parser-unreachable branch. Preserve byte-offset
behavior for quoted and multibyte names and the existing name.len()
calculations.
In `@crates/ry-analysis/tests/property.rs`:
- Around line 34-52: Update the live-versus-fresh property reconstruction to
preserve separate open-file and disk-file inputs from the live host instead of
installing every path as Change::SetDiskFile. Extend arb_change to generate
AddRoot, RemoveRoot, and SetConfig operations, and ensure the fresh host is
rebuilt with the same final roots, configuration, open files, and disk files as
the live host.
- Around line 122-134: Update the revision_monotonically_increases property test
so the loop uses a per-iteration index when constructing the SetDiskFile
content, rather than the total count n. Preserve the existing number of apply
calls and revision assertion while ensuring each iteration writes distinct
content.
In `@crates/ry-checker/src/packages.rs`:
- Around line 7-10: Add the public FFI_PRIMITIVES re-export to the
ry_checker::packages shim alongside the existing package symbols, preserving the
current re-exports and compatibility with callers that access this constant
through the checker namespace.
In `@crates/ry-cli/src/main.rs`:
- Around line 917-922: Change the parsed-file collection to store
Arc<SourceFile> values instead of owned SourceFile instances, then update the
loop around analysis_files and comments to clone the Arc rather than
deep-cloning the file. Preserve the existing path and comments handling, and
pass parsed Arc values as &SourceFile via as_ref() where ResolutionEnvironment
requires that reference.
In `@crates/ry-lsp/Cargo.toml`:
- Line 16: Remove the unused ry-analysis dependency declaration from the ry-lsp
crate’s Cargo.toml, leaving the remaining dependency configuration unchanged.
In `@crates/ry-lsp/src/backend.rs`:
- Around line 2449-2478: The fallback in the publish loop must not borrow
filter, confidence, or excludes from `state.folder_contexts.first()`. Precompute
and store root-level values from `state.file_config` during initialization and
reload, then use those values in the `folder_context_for_path` `None` branch
while preserving the existing folder-specific path and flat compile-counter
behavior. Keep the root-level fallback consistent with
`effective_baseline_for_path` and `state.root`.
- Around line 1413-1435: Update the project-aware hover fallback around
found_project_fn to search other open documents before checking
state.disk_files, matching the open-document precedence used by goto_definition.
Return the function hover when file_has_function finds the identifier in an open
document, while retaining the disk-file search as the fallback.
- Around line 3030-3059: Move the “Check if a parsed file defines a function
with the given name” doc comment onto file_has_function, and move “Extract all
top-level function names from a parsed file” onto extract_function_names. Remove
both misplaced comments from extract_prefix and extract_function_params, leaving
each function documented by its corresponding description.
- Around line 217-270: Remove the redundant file_config parameter from
compute_folder_filter and use the single config argument for both configuration
lookups and Excludes::from_config; update both callers to pass only &config and
&folder_settings. Refactor the #[cfg(test)] merge_filter helper to delegate to
compute_folder_filter instead of duplicating filter-merge logic, preserving its
existing test-facing return behavior.
- Around line 1735-1754: Make all three disk-file fallback searches in
backend.rs deterministic: in the signature-help fallback around lines 1735-1754,
the hover check around lines 1413-1435, and the go-to-definition search around
lines 1467-1505, collect and sort candidate paths before iterating. Preserve
each handler’s existing matching behavior while ensuring duplicate definitions
consistently select the same result.
- Around line 350-361: Restrict the full recollection trigger around
force_full_recollection to newly added or re-added paths that are absent from
self.files, rather than ordinary version or content changes, so incremental
checking remains effective after edits. Reuse the per-file change classification
from the following loop instead of recomputing the any_changed predicate
separately, and preserve recovery for close/reopen convergence cases.
- Around line 200-211: Move FILTER_COMPILE_COUNT and COMPILE_DURING_LAST_PUBLISH
out of process-global statics into per-Backend state, such as Arc<AtomicU64>
fields on State. Update the publish_diagnostics measurement and filter
compilation paths to use that server-owned state, and provide a per-server
accessor for tests so publish assertions only observe the current Backend.
- Around line 1894-1911: Pass each indexed disk file’s source text to the
position-mapping calls so unopened-file locations retain their real UTF-16
ranges: in crates/ry-lsp/src/backend.rs lines 1894-1911, use disk_file.source
for find_references_in_file; in lines 1492-1504, use the indexed file’s source
for find_definition_locations; and in lines 1557-1577, replace disk_text with
that source and update the comment stating text is unnecessary.
- Around line 1673-1696: The completion merge is quadratic and uses an
inconsistent definition of project-defined functions. In the completion path,
build a HashSet<String> of existing item labels once before iterating disk_files
and use it for duplicate checks while adding new labels. Consolidate
extract_function_names, file_has_function, and signature-help discovery around
one shared function-definition traversal so completion, hover, and signature
help expose the same project functions without incorrectly including nested
locals.
- Around line 3032-3054: Update extract_prefix to use the existing
position_to_byte_offset_pos conversion for the cursor position, preserving
UTF-16 handling and invalid-position behavior. Use the returned byte offset
directly as the slice boundary so the character at the cursor is excluded while
all preceding characters, including a trailing underscore, remain in the
extracted prefix.
In `@crates/ry-lsp/src/lib.rs`:
- Line 154: Replace the public re-export of COMPILE_DURING_LAST_PUBLISH and
FILTER_COMPILE_COUNT with read-only accessor functions in the backend module,
following the existing baseline_disk_reads() pattern. Re-export the accessors
from lib.rs and keep the AtomicU64 statics private so consumers can read values
without mutating the test-gate state; ensure the accessors support the requested
per-server scoping.
In `@crates/ry-lsp/tests/p38_feature_diff.rs`:
- Around line 1-16: Update the module documentation in the P38
feature-differential test module to describe these as active regression tests
for the implemented cross-file behavior, removing the claim that they are
#[ignore]’d or not yet implemented. Revise the section comments near the tests
at the referenced finding markers to describe the coverage as currently enforced
rather than future work that will fix each finding.
- Around line 32-44: Replace the unconditional one-second sleep in spawn_session
with bounded polling of the indexing completion signal. Repeatedly issue the
relevant observable request, or await the first textDocument/publishDiagnostics
notification for an indexed file, until it returns a non-null result; enforce a
deadline and fail clearly if indexing does not complete in time.
- Around line 176-191: Update the definition-result assertions around locations
and target_uris to require at least one returned location, then verify that a
returned URI equals b_uri. Preserve the existing exclusion of a_uri while
ensuring the test fails when textDocument/definition returns null or an empty
array.
In `@crates/ry-lsp/tests/w8_session.rs`:
- Around line 266-292: Update Operation::DeleteFile validity in is_valid to
allow deletion of open files by removing the !self.is_open(*file) requirement,
and update its valid_alternative selection to permit open files as deletion
candidates. In the invalid IncrementalEdit alternative handling, preserve
Operation::IncrementalEdit rather than converting it to Operation::FullEdit,
while continuing to substitute only the file slot.
- Around line 580-593: Separate the invalid-operation accounting in the
operations loop from the `skipped_count` variable into distinct counts for
replacements and dropped operations: increment the replacement count only when
`valid_alternative` returns `Some`, and increment the dropped count before
`continue` for `None`. Update the report around the existing reporting logic
near `valid_alternative` to include both outcomes with accurate labels, and
remove the unintended whitespace from the multiline string so it does not output
padding between words.
In `@crates/ry-workspace/src/file_kind.rs`:
- Around line 27-30: Update the component collection in the path-classification
function around relative.components() to reject the entire path when any
component cannot be converted to UTF-8, rather than silently removing it with
filter_map. Propagate the rejection through the existing classification result
so callers such as workspace discovery skip the path, while preserving normal
matching for fully UTF-8 paths.
In `@crates/ry-workspace/src/packages.rs`:
- Around line 234-293: Extend the tests module with coverage for
attached_packages and the remaining namespace_metadata arms. Add tests proving
attached_packages finds library/require calls in nested scopes while excluding
requireNamespace, and add a namespace_metadata test covering import, export, and
S3method results using the existing metadata fields and HashSet comparisons.
In `@docs/architecture/analysis-query-engine.md`:
- Around line 71-75: Update the rationale in the architecture document to remove
the volatile “960 tests pass” count or replace it with a single dated
test-command result consistent with the acceptance records, including the known
pre-existing failure if applicable.
In `@docs/architecture/p38-progress.md`:
- Around line 38-49: Add the text language identifier to the fenced graph blocks
around the dependency diagrams, including the block near line 130, and insert
blank lines immediately before and after each fence. Preserve the diagram
contents while ensuring all affected fences satisfy MD031 and MD040.
In `@docs/architecture/p38-w12-acceptance.md`:
- Around line 32-33: Insert a blank line between the “## Gates” heading and the
following workspace-tests list item in the acceptance document, preserving the
existing heading and content.
- Around line 12-15: The Plan 38 acceptance records incorrectly indicate
completion while W11-W12 criteria remain unfinished. In
docs/architecture/p38-w12-acceptance.md lines 12-15, retain the unchecked
criteria and mark the plan partial until implementation passes; in
docs/architecture/plans-37-39-acceptance.md lines 20-34, remove the 12/12
completion claim until compatibility removal and final acceptance are complete.
- Around line 32-35: Publish one current, dated gate result across all
acceptance records: update docs/architecture/p38-w12-acceptance.md lines 32-35,
docs/architecture/analysis-query-engine.md lines 71-75, and
docs/architecture/plans-37-39-acceptance.md lines 49-53 to consistently use the
correct 960, 963, or 970 test total and reflect convergence after commit
996348125525a8484d1d10d704d2eaf5b9401b14; update docs/release-runbook.md lines
10-18 to reference that same dated gate output and preserve the existing
evidence that both lanes pass.
In `@docs/architecture/p39-acceptance.md`:
- Around line 20-26: Add blank lines immediately after each affected Markdown
section heading at the referenced locations, including the heading above the
schema-crate list, while preserving all existing content and list structure.
In `@docs/architecture/p39-w1-catalog-design.md`:
- Around line 76-89: Update the “ry catalog adapter” contract to match the
implemented API: reference catalog_adapter.rs as the adapter and catalog.rs as
the types/trait definition, change lookup to return Option<&FunctionSemantics>,
change package_functions to return Vec<&str>, and document the missing
function_count() method before treating this section as the architecture
reference.
In `@docs/architecture/plans-37-39-acceptance.md`:
- Around line 3-5: Add a blank line immediately after each section heading at
the referenced locations in the acceptance document, including the Baseline
heading and the headings at lines 49, 55, and 64, while preserving the existing
content.
In `@docs/release-runbook.md`:
- Around line 20-21: Update the “Clean-checkout validated” step in the release
runbook to remove the git clean/cargo build manual command and instead reference
the archive-based validation procedure in
docs/architecture/project-remediation-plan.md, using git archive followed by
cargo metadata and cargo check.
- Around line 34-38: Update the “Tag format” fenced block in the release runbook
to declare the text language on its opening fence, preserving the existing
tag-format example.
In `@ecosystem/check-cargo-edges.py`:
- Around line 18-23: Update the subprocess.run invocation in the cargo
dependency-check flow to include the --locked argument, then validate
result.returncode before assigning or parsing result.stdout. On failure, report
result.stderr and terminate the check with a failure status instead of treating
empty output as a dependency-free tree.
In `@editors/code/src/common/binary.ts`:
- Around line 99-115: Update the activation/version-validation flow around
getRyVersion and checkVersionCapability so the minimum-version check is
performed even when probing returns undefined due to execution, timeout, JSON,
or parsing failures. Ensure unknown-version handling follows an explicit policy,
rather than allowing activation to proceed unverified; preserve normal
validation for successfully detected versions.
- Around line 38-67: The restart flow in runServer must not reuse
activation-time resolvedBinary.path after trust, ry.path, or importStrategy
changes. Before every startServer call, re-resolve the binary using
findRyBinaryPath, probe its version, and replace resolvedBinary with the
refreshed result, following the existing resolution logic in the activation
setup.
- Around line 50-54: Update the candidate resolution loop around resolveHomeDir
to accept only existing regular files with platform-appropriate execute
permission, skipping directories and non-runnable candidates so resolution
continues to later configured or PATH candidates and the bundled fallback. Apply
the same validation consistently to configured and PATH candidate handling.
In `@editors/code/src/common/commands.ts`:
- Around line 93-99: Update the document-opening flow in the surrounding command
handler to await both vscode.workspace.openTextDocument and
vscode.window.showTextDocument inside the existing try block. Replace the
promise callback with sequential awaits so either rejection is handled by the
existing catch block.
- Around line 67-90: Replace both execSync calls in the rule-list and
selected-rule explanation flow with execFileSync, passing binaryPath separately
from the command arguments in arrays and explicitly setting shell: false.
Preserve the existing encoding and timeout options, and keep the picked.label
value as an argument rather than interpolating it into a shell command.
In `@editors/code/src/common/environment.ts`:
- Around line 33-43: Update the PATH handling around currentPath and
collection.prepend: split currentPath using the platform separator and check for
an exact entry matching this.pathToAdd, then prepend only this.pathToAdd plus
the separator rather than the full newPath. Preserve the existing debug message,
early return, and prepend options.
In `@editors/code/src/extension.ts`:
- Around line 149-162: Before each requestRestart call in the
configuration-change and onDidGrantWorkspaceTrust handlers, reload current
workspace settings, call findRyBinaryPath using the current trust state,
re-probe and validate the binary version, and assign the refreshed result to
resolvedBinary so runServer uses the new path. Add coverage for both settings
and workspace-trust transitions.
In `@editors/code/src/test/binary.test.ts`:
- Line 11: Add Bun’s type declarations to the test TypeScript configuration used
by tsconfig.test.json, ensuring bun-types is declared as a project dependency if
needed. Keep the pretest compiler path and runtime test setup aligned so
bun:test resolves successfully without disrupting the existing Mocha E2E tests.
In `@editors/code/src/test/e2e.test.ts`:
- Around line 44-75: The test “Server starts from the resolved binary path
(P37-W2 no split-brain)” must directly verify the binary-path contract instead
of relying on diagnostics from a previously opened fixture. Isolate the test
with a distinct resolved executable, start the server through the relevant
startup path, and assert that the LanguageClient launch command receives that
exact path; remove the indirect diagnostics-based assertion.
In `@editors/zed/src/lib.rs`:
- Around line 129-140: Update the release verification flow around
verify_checksum to bind verification to the downloaded artifact: verify the
archive before extraction, or supply and validate a checksum for the extracted
binary, rejecting missing, malformed, and mismatched sidecars. Remove the unused
_has_sha256_asset logic, update the mismatch test at editors/zed/src/lib.rs
lines 450-469 to call verify_checksum with a temporary file, and adjust
editors/zed/extension.toml lines 9-12 only as needed to provide the checksum
artifact expected by the corrected flow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b92560bf-c3e9-4f09-8909-f1e1bc69106e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (79)
.github/workflows/build-vscode.yml.github/workflows/ecosystem.yml.github/workflows/release-vscode.yml.github/workflows/test-code.ymlCHANGELOG.mdCargo.tomlcrates/ry-analysis/Cargo.tomlcrates/ry-analysis/src/catalog.rscrates/ry-analysis/src/catalog_adapter.rscrates/ry-analysis/src/check.rscrates/ry-analysis/src/effect.rscrates/ry-analysis/src/interactive.rscrates/ry-analysis/src/layering.rscrates/ry-analysis/src/lib.rscrates/ry-analysis/src/rules.rscrates/ry-analysis/src/snapshot.rscrates/ry-analysis/src/symbols.rscrates/ry-analysis/tests/property.rscrates/ry-checker/Cargo.tomlcrates/ry-checker/src/cache.rscrates/ry-checker/src/diagnostics.rscrates/ry-checker/src/infer/call.rscrates/ry-checker/src/infer/misc.rscrates/ry-checker/src/lib.rscrates/ry-checker/src/packages.rscrates/ry-checker/src/project.rscrates/ry-checker/src/semantic_lists.rscrates/ry-checker/src/tests.rscrates/ry-checker/tests/vendor_snapshot.rscrates/ry-cli/Cargo.tomlcrates/ry-cli/src/main.rscrates/ry-config/Cargo.tomlcrates/ry-config/src/baseline.rscrates/ry-core/Cargo.tomlcrates/ry-core/src/diagnostic.rscrates/ry-core/src/lib.rscrates/ry-core/src/parser.rscrates/ry-core/tests/parser_correctness.rscrates/ry-lsp/Cargo.tomlcrates/ry-lsp/src/backend.rscrates/ry-lsp/src/lib.rscrates/ry-lsp/tests/p36_contract.rscrates/ry-lsp/tests/p38_feature_diff.rscrates/ry-lsp/tests/w8_session.rscrates/ry-workspace/Cargo.tomlcrates/ry-workspace/src/file_kind.rscrates/ry-workspace/src/lib.rscrates/ry-workspace/src/packages.rsdocs/architecture/analysis-query-engine.mddocs/architecture/cache-decision.mddocs/architecture/p38-progress.mddocs/architecture/p38-w12-acceptance.mddocs/architecture/p39-acceptance.mddocs/architecture/p39-w1-catalog-design.mddocs/architecture/plans-37-39-acceptance.mddocs/architecture/project-remediation-plan.mddocs/corpus/posit-0.9.0.jsondocs/editor-defaults.mddocs/release-runbook.mdecosystem/check-cargo-edges.pyecosystem/check-ledger.pyeditors/code/.vscode-test.jseditors/code/README.mdeditors/code/eslint.config.mjseditors/code/package.jsoneditors/code/src/common/binary.tseditors/code/src/common/commands.tseditors/code/src/common/constants.tseditors/code/src/common/environment.tseditors/code/src/common/server.tseditors/code/src/common/settings.tseditors/code/src/common/status.tseditors/code/src/common/version.tseditors/code/src/extension.tseditors/code/src/test/binary.test.tseditors/code/src/test/e2e.test.tseditors/code/tsconfig.test.jsoneditors/zed/extension.tomleditors/zed/src/lib.rs
💤 Files with no reviewable changes (2)
- crates/ry-workspace/Cargo.toml
- crates/ry-checker/src/cache.rs
Comment on lines
+9
to
+15
| paths: | ||
| - "editors/code/**" | ||
| - ".github/workflows/build-vscode.yml" | ||
| pull_request: | ||
| paths: | ||
| - "editors/code/**" | ||
| - ".github/workflows/build-vscode.yml" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run this workflow for Rust changes that affect ry-cli.
The job builds ry-cli at Line 40. A change to Cargo.lock, workspace manifests, crates/ry-cli, or its dependencies does not match these filters. The VSIX and E2E gate then does not run for that change. Include the Rust build inputs in paths, or remove the path filters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-vscode.yml around lines 9 - 15, Update the path
filters for the build-vscode workflow’s push and pull_request triggers to
include all Rust inputs affecting the ry-cli build, including Cargo.lock,
workspace manifests, crates/ry-cli, and its dependency crates; alternatively
remove the filters so those changes trigger the workflow. Preserve the existing
editor and workflow paths.
| with: | ||
| persist-credentials: false | ||
|
|
||
| - uses: oven-sh/setup-bun@0c5077e51419868618ae5fe8019c62421857d6 # v2 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -eu
printf '%s\n' '--- workflow references ---'
rg -n -C 2 'oven-sh/setup-bun@' .github/workflows
printf '%s\n' '--- target workflow ---'
sed -n '25,40p' .github/workflows/build-vscode.yml
printf '%s\n' '--- tag resolution ---'
python3 - <<'PY'
import json, urllib.request
repo = "oven-sh/setup-bun"
for tag in ("v2",):
url = f"https://api.github.com/repos/{repo}/git/ref/tags/{tag}"
req = urllib.request.Request(url, headers={"Accept": "application/vnd.github+json", "User-Agent": "review-verifier"})
with urllib.request.urlopen(req) as r:
ref = json.load(r)
obj = ref["object"]
print("ref:", ref["ref"])
print("object:", obj)
if obj["type"] == "tag":
url = f"https://api.github.com/repos/{repo}/git/tags/{obj['sha']}"
req = urllib.request.Request(url, headers={"Accept": "application/vnd.github+json", "User-Agent": "review-verifier"})
with urllib.request.urlopen(req) as r:
tag_obj = json.load(r)
print("dereferenced:", tag_obj["object"])
commit_sha = tag_obj["object"]["sha"]
else:
commit_sha = obj["sha"]
url = f"https://api.github.com/repos/{repo}/commits/{commit_sha}"
req = urllib.request.Request(url, headers={"Accept": "application/vnd.github+json", "User-Agent": "review-verifier"})
with urllib.request.urlopen(req) as r:
commit = json.load(r)
print("commit:", commit["sha"])
PYRepository: sims1253/ry
Length of output: 4113
🏁 Script executed:
set -eu
printf '%s\n' '--- dereferenced v2 tag ---'
git ls-remote https://github.com/oven-sh/setup-bun.git 'refs/tags/v2' 'refs/tags/v2^{}'
printf '%s\n' '--- candidate commit resolution ---'
for sha in \
0c5077e51419868618ae5fe8019c62421857d6 \
0c5077e51419868618aeaa5fe8019c62421857d6
do
printf '%s: ' "$sha"
curl -ksSfL \
-H 'Accept: application/vnd.github+json' \
-H 'User-Agent: review-verifier' \
"https://api.github.com/repos/oven-sh/setup-bun/commits/$sha" |
jq -r '.sha // ("ERROR: " + (.message // "unknown"))'
doneRepository: sims1253/ry
Length of output: 549
🏁 Script executed:
set -u
sha='0c5077e51419868618aeaa5fe8019c62421857d6'
printf '%s\n' '--- GitHub CLI availability ---'
if command -v gh >/dev/null 2>&1; then
gh api "repos/oven-sh/setup-bun/commits/$sha" --jq '.sha' || true
else
echo 'gh: unavailable'
fi
printf '%s\n' '--- direct API commit check ---'
curl --noproxy '*' -sS -k \
-H 'Accept: application/vnd.github+json' \
-H 'User-Agent: review-verifier' \
"https://api.github.com/repos/oven-sh/setup-bun/commits/$sha" |
jq -r '.sha // ("ERROR: " + (.message // "unknown"))' || trueRepository: sims1253/ry
Length of output: 427
Use the immutable setup-bun v2 commit SHA.
Replace 0c5077e51419868618ae5fe8019c62421857d6 with 0c5077e51419868618aeaa5fe8019c62421857d6.
🧰 Tools
🪛 GitHub Check: zizmor
[failure] 33-33:
unpinned action reference: action is not pinned to a hash (required by blanket policy)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-vscode.yml at line 33, Update the
`oven-sh/setup-bun` action reference in the workflow to use the requested
immutable v2 commit SHA, replacing the existing SHA with
`0c5077e51419868618ae5fe8019c62421857d6` while leaving the action version
comment unchanged.
Source: Linters/SAST tools
Comment on lines
+92
to
+93
| # Extract from README marketplace link | ||
| readme_id=$(grep -oP 'itemName=\K[^")\s]+' README.md | head -1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Read the extension README in both publisher checks.
Both jobs run from the repository root and inspect README.md. The extension marketplace identifier is in editors/code/README.md. If the root README has no match, pipefail makes the job fail. If it has another marketplace link, the gate validates the wrong document.
.github/workflows/build-vscode.yml#L92-L93: change the input path toeditors/code/README.md..github/workflows/test-code.yml#L62-L62: change the input path toeditors/code/README.md.
📍 Affects 2 files
.github/workflows/build-vscode.yml#L92-L93(this comment).github/workflows/test-code.yml#L62-L62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-vscode.yml around lines 92 - 93, Update the README
input used by the marketplace identifier extraction in
.github/workflows/build-vscode.yml lines 92-93 and
.github/workflows/test-code.yml line 62 to read editors/code/README.md instead
of the repository-root README; no other workflow behavior should change.
| run: | | ||
| set -euo pipefail | ||
| base_url="https://github.com/sims1253/ry/releases/latest/download" | ||
| base_url="https://github.com/sims1253/ry/releases/download/${{ inputs.core-tag }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pass core-tag through an environment variable.
The workflow expands inputs.core-tag directly in shell source. A crafted value can terminate the quoted assignment and run commands in the release build job. Pass the input through env and expand "$CORE_TAG" inside the script.
Proposed fix
+ env:
+ CORE_TAG: ${{ inputs.core-tag }}
run: |
set -euo pipefail
- base_url="https://github.com/sims1253/ry/releases/download/${{ inputs.core-tag }}"
+ base_url="https://github.com/sims1253/ry/releases/download/${CORE_TAG}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| base_url="https://github.com/sims1253/ry/releases/download/${{ inputs.core-tag }}" | |
| env: | |
| CORE_TAG: ${{ inputs.core-tag }} | |
| run: | | |
| set -euo pipefail | |
| base_url="https://github.com/sims1253/ry/releases/download/${CORE_TAG}" |
🧰 Tools
🪛 GitHub Check: zizmor
[failure] 80-80:
code injection via template expansion: may expand into attacker-controllable code
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-vscode.yml at line 80, Update the release build
job’s shell step around the base_url assignment to pass inputs.core-tag via the
step environment, then construct the URL using the quoted "$CORE_TAG"
environment variable instead of interpolating the input directly in shell
source.
Source: Linters/SAST tools
Comment on lines
+9
to
+11
| #### P37-W1: Parser UTF-8 boundary panic fix | ||
| - Fixed panic in `lower_namespace` when string RHS last byte is inside a | ||
| multi-byte character. Extracted shared `strip_quotes_at_boundaries` helper. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines after the subsection headings.
Line 9 is followed immediately by paragraph text. Apply the same fix after Lines 13, 19, 25, 30, 34, 39, and 44.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 9 - 11, Add blank lines immediately after each
subsection heading in CHANGELOG.md, including P37-W1 and the headings at the
other specified sections, while leaving the changelog text unchanged.
Source: Linters/SAST tools
Comment on lines
+33
to
+43
| const currentPath = collection.get("PATH")?.value ?? process.env.PATH ?? ""; | ||
| if (currentPath.includes(this.pathToAdd)) { | ||
| this.logger.debug(`${this.pathToAdd} is already on PATH`); | ||
| return; | ||
| } | ||
|
|
||
| const newPath = `${this.pathToAdd}${process.platform === "win32" ? ";" : ":"}${currentPath}`; | ||
| collection.prepend("PATH", newPath, { | ||
| applyAtShellIntegration: true, | ||
| applyAtProcessCreation: false, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="editors/code/src/common/environment.ts"
printf '%s\n' '--- candidate file outline ---'
ast-grep outline "$file" --lang typescript || true
printf '%s\n' '--- relevant source ---'
sed -n '1,120p' "$file"
printf '%s\n' '--- PATH collection API usages ---'
rg -n -C 4 'collection\.(prepend|append|replace|get)\("PATH"|pathToAdd|applyAtShellIntegration|applyAtProcessCreation' editors/code/src editors/code/test 2>/dev/null || true
printf '%s\n' '--- repository references to EnvironmentVariableCollection semantics ---'
rg -n -C 3 'EnvironmentVariableCollection|prepend\(' editors/code package.json 2>/dev/null | head -300 || trueRepository: sims1253/ry
Length of output: 7074
🏁 Script executed:
#!/bin/bash
set -eu
file="editors/code/src/common/environment.ts"
sed -n '1,120p' "$file"
rg -n -C 4 \
'collection\.(prepend|append|replace|get)\("PATH"|pathToAdd|applyAtShellIntegration|applyAtProcessCreation' \
editors/code/src editors/code/test 2>/dev/null || trueRepository: sims1253/ry
Length of output: 4706
🌐 Web query:
VS Code API EnvironmentVariableCollection prepend get mutator value semantics
💡 Result:
The EnvironmentVariableCollection.prepend method in the VS Code API is used to add a value to the beginning of an environment variable for terminals [1][2]. Key semantics and behaviors include: 1. Single Mutation Limit: An extension can make only a single change to any one specific environment variable per EnvironmentVariableCollection instance [1][2]. Subsequent calls to replace, append, or prepend for the same variable will overwrite any previous calls [1][2]. 2. Options and Timing: The method accepts an optional EnvironmentVariableMutatorOptions object [1][3]. If no options are provided, it defaults to { applyAtProcessCreation: true } [1][2]. You can control precisely when the mutation is applied by setting flags such as applyAtProcessCreation (applied when the terminal process is launched) or applyAtShellIntegration (applied via VS Code's shell integration script, which runs after shell profile scripts) [3][4][5]. 3. Shell Integration: Because environment variable modification can be complex—particularly on platforms like macOS where login shells or shell profile scripts (like .zshrc) may re-initialize or override environment variables—using applyAtShellIntegration: true is often necessary to ensure the prepended value is correctly positioned at the start of the variable [4][6][5]. 4. State: The EnvironmentVariableCollection itself is an iterable of the current mutations [3][7]. For scenarios where prepend is not behaving as expected (e.g., being overridden by shell configuration files), ensuring shell integration is enabled and using the appropriate options is the standard approach to maintaining the desired environment variable order [4][6][5].
Citations:
- 1: https://jsr.io/@narumincho/vscode/doc/~/EnvironmentVariableCollection.prepend
- 2: https://vshaxe.github.io/vscode-extern/vscode/EnvironmentVariableCollection.html
- 3: Allow EnvironmentVariableCollection API to apply changes via shell integration microsoft/vscode#179476
- 4: Prepending PATH env var with environmentVariableCollection doesn't work on macOS microsoft/vscode#99878
- 5:
.zshrcis overriding the environment variable set by shell integration on macOS microsoft/vscode#205102 - 6: PATH mutation using EnvironmentVariableCollection prepend is overwritten in zsh microsoft/vscode#188235
- 7: Allow EnvironmentVariableCollection to unset variables microsoft/vscode#185200
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- referenced upstream paths ---'
rg -n -C 5 'PathEnvironmentVariableManager|class .*Environment.*Variable|prepend\("PATH"|environment\.ts' . \
-g '*.ts' -g '*.js' -g '*.md' 2>/dev/null | head -400 || true
printf '%s\n' '--- package and lockfile VS Code API versions ---'
rg -n '"`@types/vscode`"|"vscode"' package.json editors/code/package.json editors/code/*lock* pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || trueRepository: sims1253/ry
Length of output: 3065
🏁 Script executed:
#!/bin/bash
set -eu
rg -n -C 5 \
'PathEnvironmentVariableManager|prepend\("PATH"|environment\.ts' . \
-g '*.ts' -g '*.js' -g '*.md' 2>/dev/null | head -400 || trueRepository: sims1253/ry
Length of output: 2624
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
cases = [
("/opt/ry", "/opt/ry-old:/usr/bin", ":"),
("/opt/ry", "/usr/bin:/bin", ":"),
(r"C:\ry", r"C:\ry-old;C:\Windows\System32", ";"),
]
for path_to_add, current_path, separator in cases:
substring_match = path_to_add in current_path
exact_match = path_to_add in current_path.split(separator)
prefix = path_to_add if path_to_add.endswith(separator) else path_to_add + separator
# Model EnvironmentVariableCollection.prepend: supplied value + existing PATH.
resulting_path = prefix + current_path
print({
"path_to_add": path_to_add,
"current_path": current_path,
"substring_match": substring_match,
"exact_match": exact_match,
"resulting_path_with_prefix": resulting_path,
"resulting_path_with_full_new_path": (
f"{path_to_add}{separator}{current_path}{current_path}"
),
})
PYRepository: sims1253/ry
Length of output: 962
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
cases = [
("/opt/ry", "/opt/ry-old:/usr/bin", ":"),
(r"C:\ry", r"C:\ry-old;C:\Windows\System32", ";"),
]
for path_to_add, current_path, separator in cases:
print("substring:", path_to_add in current_path)
print("exact entry:", path_to_add in current_path.split(separator))
print("prepend prefix:", path_to_add + separator + current_path)
print("prepend current implementation:", path_to_add + separator + current_path + current_path)
PYRepository: sims1253/ry
Length of output: 494
Pass only the new PATH prefix and compare PATH entries.
currentPath.includes(this.pathToAdd) can match a substring such as /opt/ry-old. Also, prepend adds its argument to the existing PATH, so passing newPath duplicates the existing entries. Split PATH by the platform separator for exact matching, then pass only this.pathToAdd followed by the separator.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editors/code/src/common/environment.ts` around lines 33 - 43, Update the PATH
handling around currentPath and collection.prepend: split currentPath using the
platform separator and check for an exact entry matching this.pathToAdd, then
prepend only this.pathToAdd plus the separator rather than the full newPath.
Preserve the existing debug message, early return, and prepend options.
Comment on lines
+149
to
+162
| const oldSettings = settings; | ||
| const newSettings = getWorkspaceSettings(serverId, { | ||
| uri: vscode.Uri.file(process.cwd()), | ||
| index: 0, | ||
| name: "root", | ||
| } as vscode.WorkspaceFolder); | ||
| const binaryPath = findRyBinaryPath(settings, isUntrusted); | ||
| const version = getRyVersion(binaryPath); | ||
| resolvedBinary = { path: binaryPath, version }; | ||
|
|
||
| if (version) { | ||
| const versionError = checkVersionCapability( | ||
| resolvedBinary, | ||
| MINIMUM_VERSION, | ||
| "settings channel", | ||
| ); | ||
| if (versionError) { | ||
| statusItem.setError(versionError); | ||
| vscode.window.showErrorMessage(versionError); | ||
| return; | ||
| } | ||
| } as vscode.WorkspaceFolder); | ||
|
|
||
| if (checkIfConfigurationChanged(oldSettings, newSettings)) { | ||
| await requestRestart(); | ||
| } | ||
| }), | ||
| // E3: Workspace trust changes respawn because trust affects binary resolution. | ||
| vscode.workspace.onDidGrantWorkspaceTrust(async () => { | ||
| await requestRestart(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Refresh resolvedBinary before a restart.
Lines 149-162 request a restart but retain the activation-time settings and resolvedBinary. runServer therefore launches the old path at Line 99 after ry.path, ry.importStrategy, or workspace trust changes.
Reload workspace settings, call findRyBinaryPath with the current trust state, re-probe and validate the version, then replace resolvedBinary before calling requestRestart. Add coverage for a settings or trust transition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editors/code/src/extension.ts` around lines 149 - 162, Before each
requestRestart call in the configuration-change and onDidGrantWorkspaceTrust
handlers, reload current workspace settings, call findRyBinaryPath using the
current trust state, re-probe and validate the binary version, and assign the
refreshed result to resolvedBinary so runServer uses the new path. Add coverage
for both settings and workspace-trust transitions.
| * to execute an arbitrary binary. | ||
| */ | ||
|
|
||
| import { describe, it, expect } from "bun:test"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files 'editors/code/src/test/binary.test.ts' 'editors/code/**/tsconfig*.json' 'editors/code/**/package.json' 'editors/code/**/bun.lock*' 'editors/code/**/vitest*' 'editors/code/**/jest*'
printf '%s\n' '--- test file ---'
cat -n editors/code/src/test/binary.test.ts
printf '%s\n' '--- TypeScript and package configuration ---'
for f in $(git ls-files 'editors/code/**/tsconfig*.json' 'editors/code/**/package.json' 'editors/code/**/bun.lock*' 'editors/code/**/vitest*' 'editors/code/**/jest*'); do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '%s\n' '--- references to test runners and test types ---'
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' \
'bun:test|bun-types|vitest|jest|`@types/jest`|describe|tsconfig\.test|tsc -p' \
editors/codeRepository: sims1253/ry
Length of output: 3722
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files \
'editors/code/src/test/binary.test.ts' \
'editors/code/**/tsconfig*.json' \
'editors/code/**/package.json' \
'editors/code/**/bun.lock*' \
'editors/code/**/vitest*' \
'editors/code/**/jest*'
printf '%s\n' '--- test file ---'
cat -n editors/code/src/test/binary.test.ts
printf '%s\n' '--- TypeScript and package configuration ---'
while IFS= read -r f; do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done < <(git ls-files \
'editors/code/**/tsconfig*.json' \
'editors/code/**/package.json' \
'editors/code/**/bun.lock*' \
'editors/code/**/vitest*' \
'editors/code/**/jest*')
printf '%s\n' '--- references to test runners and test types ---'
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' \
'bun:test|bun-types|vitest|jest|`@types/jest`|describe|tsconfig\.test|tsc -p' \
editors/codeRepository: sims1253/ry
Length of output: 3369
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- editors/code directory ---'
find editors/code -maxdepth 3 -type f -print | sort
printf '%s\n' '--- repository configuration files ---'
find . -maxdepth 4 -type f \( \
-name 'package.json' -o \
-name 'tsconfig*.json' -o \
-name 'bunfig.toml' -o \
-name 'bun.lock' -o \
-name 'bun.lockb' -o \
-name 'pnpm-lock.yaml' -o \
-name 'yarn.lock' -o \
-name 'package-lock.json' \
\) -print | sort
printf '%s\n' '--- package.json ---'
if [ -f editors/code/package.json ]; then
cat -n editors/code/package.json
fi
printf '%s\n' '--- relevant repository references ---'
rg -n --hidden -g '!node_modules' -g '!dist' -g '!build' \
'tsconfig\.test\.json|bun:test|bun-types|bun test|compile-tests|`@types/node`|typescript|test runner|vitest|jest' \
.Repository: sims1253/ry
Length of output: 17849
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- TypeScript configurations ---'
cat -n editors/code/tsconfig.json
printf '\n--- editors/code/tsconfig.test.json ---\n'
cat -n editors/code/tsconfig.test.json
printf '%s\n' '--- VS Code workflow ---'
cat -n .github/workflows/build-vscode.yml
printf '%s\n' '--- relevant documentation ---'
sed -n '70,110p' docs/architecture/project-remediation-plan.mdRepository: sims1253/ry
Length of output: 6098
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- E2E test imports and declarations ---'
cat -n editors/code/src/test/e2e.test.ts
printf '%s\n' '--- Bun-related lockfile entries ---'
rg -n -C 2 '"bun-types"|"`@types/bun`"|bun:test' editors/code/bun.lock
printf '%s\n' '--- read-only configuration probe ---'
python3 - <<'PY'
import json
from pathlib import Path
root = Path("editors/code")
package = json.loads((root / "package.json").read_text())
base = json.loads((root / "tsconfig.json").read_text())
tests = json.loads((root / "tsconfig.test.json").read_text())
test_files = sorted(str(p.relative_to(root)) for p in (root / "src/test").glob("**/*.ts"))
bun_imports = []
for name in test_files:
text = (root / name).read_text()
if 'from "bun:test"' in text or "from 'bun:test'" in text:
bun_imports.append(name)
print("compile-tests:", package["scripts"]["compile-tests"])
print("pretest:", package["scripts"]["pretest"])
print("base types:", base["compilerOptions"]["types"])
print("test types:", tests["compilerOptions"]["types"])
print("test include:", tests["include"])
print("test files:", test_files)
print("bun:test files:", bun_imports)
print("bun-types in devDependencies:", "bun-types" in package["devDependencies"])
print("bun-types in lockfile:", '"bun-types"' in (root / "bun.lock").read_text())
print("mocha types in devDependencies:", "`@types/mocha`" in package["devDependencies"])
PYRepository: sims1253/ry
Length of output: 3321
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
from pathlib import Path
root = Path("editors/code")
package = json.loads((root / "package.json").read_text())
base = json.loads((root / "tsconfig.json").read_text())
tests = json.loads((root / "tsconfig.test.json").read_text())
lock = (root / "bun.lock").read_text()
test_files = sorted(
str(p.relative_to(root))
for p in (root / "src/test").glob("**/*.ts")
)
bun_imports = [
name for name in test_files
if 'from "bun:test"' in (root / name).read_text()
or "from 'bun:test'" in (root / name).read_text()
]
print("compile-tests:", package["scripts"]["compile-tests"])
print("pretest:", package["scripts"]["pretest"])
print("base types:", base["compilerOptions"]["types"])
print("test types:", tests["compilerOptions"]["types"])
print("test include:", tests["include"])
print("test files:", test_files)
print("bun:test files:", bun_imports)
print("bun-types in devDependencies:", "bun-types" in package["devDependencies"])
print("bun-types in lockfile:", '"bun-types"' in lock)
print("mocha types in devDependencies:", "`@types/mocha`" in package["devDependencies"])
PY
printf '%s\n' '--- Bun-related lockfile references ---'
if rg -n -C 2 '"bun-types"|"`@types/bun`"|bun:test' editors/code/bun.lock; then
:
else
printf '%s\n' 'No Bun type package or bun:test reference found in editors/code/bun.lock.'
fiRepository: sims1253/ry
Length of output: 705
Add Bun test types to the test configuration.
tsconfig.test.json includes src/test/binary.test.ts, but its types list excludes Bun and the project does not declare bun-types. The pretest script runs tsc -p tsconfig.test.json, so bun:test causes TS2307 before the VS Code tests run. Add the Bun types, or compile the Bun unit test separately from the Mocha E2E tests. Keep the runtime and TypeScript test configurations aligned.
🧰 Tools
🪛 GitHub Actions: Test VS Code extension / 1_test.txt
[error] 11-11: TypeScript compilation failed during 'bun run test' (tsc -p tsconfig.test.json --outDir out): Cannot find module 'bun:test' or its corresponding type declarations (TS2307).
🪛 GitHub Actions: Test VS Code extension / test
[error] 11-11: TypeScript compilation failed during 'bun run test' (tsc -p tsconfig.test.json --outDir out): Cannot find module 'bun:test' or its corresponding type declarations (TS2307).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editors/code/src/test/binary.test.ts` at line 11, Add Bun’s type declarations
to the test TypeScript configuration used by tsconfig.test.json, ensuring
bun-types is declared as a project dependency if needed. Keep the pretest
compiler path and runtime test setup aligned so bun:test resolves successfully
without disrupting the existing Mocha E2E tests.
Source: Pipeline failures
Comment on lines
+44
to
+75
| // P37-W2: After the split-brain fix, startServer() receives the | ||
| // pre-resolved binaryPath from extension.ts (which called | ||
| // findRyBinaryPath with the isUntrusted flag). The server starting | ||
| // and producing diagnostics proves a single binary identity — no | ||
| // separate resolveBinary() path can launch a different binary. | ||
| // The unit tests in binary.test.ts verify trust-honoring resolution. | ||
| it("Server starts from the resolved binary path (P37-W2 no split-brain)", async function () { | ||
| this.timeout(30000); | ||
|
|
||
| const fixturePath = path.join( | ||
| __dirname, | ||
| "..", | ||
| "..", | ||
| "testFixture", | ||
| "bad.R", | ||
| ); | ||
| const uri = vscode.Uri.file(fixturePath); | ||
| const doc = await vscode.workspace.openTextDocument(uri); | ||
| await vscode.window.showTextDocument(doc); | ||
|
|
||
| const deadline = Date.now() + 15000; | ||
| let serverStarted = false; | ||
| while (Date.now() < deadline) { | ||
| const diags = vscode.languages.getDiagnostics(uri); | ||
| if (diags.length > 0) { | ||
| serverStarted = true; | ||
| break; | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, 200)); | ||
| } | ||
| // Server started means startServer received a valid binaryPath. | ||
| expect(serverStarted).to.equal(true); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift
Test the binary-path contract directly.
This test only observes diagnostics for a fixture opened by the preceding test. Existing diagnostics can make it pass without a new server start. A different executable can also emit diagnostics, so this does not verify the no-split-brain contract.
Use an isolated test with a distinct resolved executable and assert the LanguageClient launch command receives that exact path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editors/code/src/test/e2e.test.ts` around lines 44 - 75, The test “Server
starts from the resolved binary path (P37-W2 no split-brain)” must directly
verify the binary-path contract instead of relying on diagnostics from a
previously opened fixture. Isolate the test with a distinct resolved executable,
start the server through the relevant startup path, and assert that the
LanguageClient launch command receives that exact path; remove the indirect
diagnostics-based assertion.
Comment on lines
+129
to
+140
| let sha256_asset_name = format!("{}.sha256", release_details.asset_name); | ||
| let _has_sha256_asset = release | ||
| .assets | ||
| .iter() | ||
| .any(|a| a.name == sha256_asset_name); | ||
| match Self::verify_checksum(None, &release_details.downloaded_binary_path) { | ||
| Ok(()) => {} | ||
| Err(error) => { | ||
| // Remove the partial download on verification failure. | ||
| let _ = fs::remove_dir_all(&release_details.downloaded_directory); | ||
| return Err(format!( | ||
| "Binary checksum verification failed: {error}. The download may be corrupted or tampered with." |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline editors/zed/src/lib.rs --items all
rg -n -C 5 'download_file|sha256_asset|verify_checksum|downloaded_binary_path' editors/zed/src/lib.rs
rg -n -C 6 'sha256|checksum|upload.*asset|release' .github/workflows ecosystemRepository: sims1253/ry
Length of output: 40773
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- download and checksum flow ---'
sed -n '90,170p' editors/zed/src/lib.rs
printf '%s\n' '--- checksum implementation and tests ---'
sed -n '223,285p' editors/zed/src/lib.rs
sed -n '416,475p' editors/zed/src/lib.rs
printf '%s\n' '--- Zed extension metadata ---'
cat editors/zed/extension.toml
printf '%s\n' '--- release asset generation and naming ---'
sed -n '70,112p' .github/workflows/release-vscode.yml
rg -n -C 4 'sha256|checksum|dist.*hash|hash.*asset|\.sha256' .github editors/zedRepository: sims1253/ry
Length of output: 50367
Bind the release checksum to the downloaded artifact before executing it.
_has_sha256_asset is unused, and verify_checksum(None, ...) skips checksum comparison. A modified binary can therefore pass the check. The mismatch test also does not call verify_checksum.
The release pipeline provides checksums for the downloaded archive, while this code hashes the extracted binary. Verify the archive before extraction, or provide a checksum for the extracted binary. Reject missing, malformed, and mismatched sidecars. Add a test that calls verify_checksum with a temporary file.
📍 Affects 2 files
editors/zed/src/lib.rs#L129-L140(this comment)editors/zed/src/lib.rs#L450-L469editors/zed/extension.toml#L9-L12
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editors/zed/src/lib.rs` around lines 129 - 140, Update the release
verification flow around verify_checksum to bind verification to the downloaded
artifact: verify the archive before extraction, or supply and validate a
checksum for the extracted binary, rejecting missing, malformed, and mismatched
sidecars. Remove the unused _has_sha256_asset logic, update the mismatch test at
editors/zed/src/lib.rs lines 450-469 to call verify_checksum with a temporary
file, and adjust editors/zed/extension.toml lines 9-12 only as needed to provide
the checksum artifact expected by the corrected flow.
Comment on lines
+26
to
+32
| fn convert_param(p: &ry_typeshed::ParamSpec) -> ParameterSpec { | ||
| ParameterSpec { | ||
| name: p.name.clone(), | ||
| has_default: p.default.is_some(), | ||
| variadic: p.name == "...", | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine the schema meaning of ParamSpec.default and ParamSpec.required.
set -euo pipefail
rg -nP --type=rust -C 10 'struct\s+ParamSpec' crates/ry-typeshed/src/lib.rs
# Find existing consumers that already interpret these two fields.
rg -nP --type=rust -C 4 '\.required\b|\.default\b' crates/ry-checker/src crates/ry-typeshed/srcRepository: sims1253/ry
Length of output: 17316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- catalog adapter and ParameterSpec definitions/usages ---'
rg -n -C 8 'fn convert_param|struct ParameterSpec|has_default|RY091|emit_missing' \
crates/ry-analysis crates/ry-checker crates/ry-typeshed
printf '%s\n' '--- ParamSpec deserialization and validation ---'
sed -n '500,565p' crates/ry-typeshed/src/lib.rs
sed -n '1200,1240p' crates/ry-typeshed/src/lib.rs
sed -n '1685,1730p' crates/ry-typeshed/src/lib.rs
printf '%s\n' '--- relevant catalog adapter source ---'
sed -n '1,80p' crates/ry-analysis/src/catalog_adapter.rsRepository: sims1253/ry
Length of output: 44699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- schema documentation and fixture values ---'
rg -n -i -C 4 'required|default' \
docs crates/ry-typeshed testdata 2>/dev/null | head -n 500
printf '%s\n' '--- all ParamSpec default/required combinations in tracked data ---'
rg -n -o '"required"[[:space:]]*:[[:space:]]*(true|false)|"default"[[:space:]]*:[[:space:]]*(true|false|null)' \
. --glob '*.json' --glob '*.md' --glob '*.rs' --glob '*.toml' 2>/dev/null |
head -n 500
printf '%s\n' '--- catalog adapter consumers and has_default call graph ---'
rg -n -C 10 'convert_function_sig|ParameterSpec|has_default' \
crates --glob '*.rs'Repository: sims1253/ry
Length of output: 44202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracked typeshed files ---'
git ls-files | grep -E '(^|/)(typeshed|stub|schema|catalog)|ry-typeshed' | head -n 200
printf '%s\n' '--- exact default boolean values in tracked source/data ---'
rg -n --glob '*.json' --glob '*.rs' --glob '*.md' \
'"default"[[:space:]]*:[[:space:]]*(true|false)' . 2>/dev/null |
grep -E '"default"[[:space:]]*:[[:space:]]*(true|false)' |
head -n 200 || true
printf '%s\n' '--- exact required/default object pairs ---'
python3 - <<'PY'
import json
from pathlib import Path
for path in Path('.').rglob('*.json'):
try:
value = json.loads(path.read_text())
except Exception:
continue
def walk(x):
if isinstance(x, dict):
if 'name' in x and ('required' in x or 'default' in x):
print(path, {k: x[k] for k in ('name', 'required', 'default') if k in x})
for v in x.values():
walk(v)
elif isinstance(x, list):
for v in x:
walk(v)
walk(value)
PY
printf '%s\n' '--- all has_default references ---'
rg -n -C 6 'has_default' crates --glob '*.rs' || trueRepository: sims1253/ry
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracked typeshed files ---'
git ls-files | grep -E '(^|/)(typeshed|stub|schema|catalog)|ry-typeshed' | head -n 200 || true
printf '%s\n' '--- exact default boolean values in tracked source/data ---'
rg -n --glob '*.json' --glob '*.rs' --glob '*.md' \
'"default"[[:space:]]*:[[:space:]]*(true|false)' . 2>/dev/null |
head -n 200 || true
printf '%s\n' '--- exact required/default object pairs ---'
python3 - <<'PY'
import json
from pathlib import Path
for path in Path('.').rglob('*.json'):
try:
value = json.loads(path.read_text())
except Exception:
continue
def walk(x):
if isinstance(x, dict):
if 'name' in x and ('required' in x or 'default' in x):
print(path, {k: x[k] for k in ('name', 'required', 'default') if k in x})
for v in x.values():
walk(v)
elif isinstance(x, list):
for v in x:
walk(v)
walk(value)
PY
printf '%s\n' '--- all has_default references ---'
rg -n -C 6 'has_default' crates --glob '*.rs' || trueRepository: sims1253/ry
Length of output: 50367
🌐 Web query:
sims1253/ry ParamSpec default required Option<bool> typeshed schema
💡 Result:
Based on your query, there are two distinct projects named "ry" and the technical details regarding ParamSpec defaults, required status, and Option typeshed schemas appear to be unrelated to the specific R language static checker "sims1253/ry" [1]. 1. sims1253/ry: This is a fast static checker for the R language, written in Rust [1]. It does not involve Python's ParamSpec or typeshed schemas in the context of its own architecture [1]. Its configuration is handled via a ry.toml file [1]. 2. Python ParamSpec Defaults (PEP 696): The concept of "ParamSpec defaults" is a Python feature introduced by PEP 696 [2]. In Python's typeshed (the collection of type stubs for the standard library), ParamSpec is defined in typing.pyi [3][4][5]. As of recent Python versions (3.13+), ParamSpec includes a default attribute [3][4]. 3. Rust ParamSpec (leviath-scripting): There is an unrelated Rust project (leviath-scripting) that contains a ParamSpec struct with a schema: Option<Value> field, a required: bool field, and a description: String field [6]. This appears to be the source of the "Option" and "schema" terminology in your query [6]. In summary, your search terms combine an R language static checker [1], a Python type system feature [2], and a Rust-based tool definition [6]. There is no intersection between the "sims1253/ry" project and the other technical concepts mentioned.
Citations:
- 1: https://github.com/sims1253/ry
- 2: https://peps.python.org/pep-0696/
- 3: https://github.com/python/typeshed/blob/master/stdlib/typing.pyi
- 4: https://github.com/python/mypy/blob/52de0c73/mypy/typeshed/stdlib/typing.pyi
- 5: https://github.com/python/typeshed/blob/2c534dc2208191dc1ae1a6f51554fa5022cbfa0a/stdlib/typing.pyi
- 6: https://docs.rs/leviath-scripting/latest/leviath_scripting/tool/struct.ParamSpec.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository metadata and schema-related files ---'
curl -fsSL https://api.github.com/repos/sims1253/ry/git/trees/main?recursive=1 |
jq -r '.tree[].path' |
grep -Ei 'schema|typeshed|catalog|readme|json' |
head -n 200
printf '%s\n' '--- upstream references to ParamSpec/default semantics ---'
for path in \
crates/ry-typeshed/src/lib.rs \
crates/ry-analysis/src/catalog_adapter.rs \
crates/ry-analysis/src/catalog.rs \
crates/ry-typeshed/vendor/SOURCE \
README.md
do
url="https://raw.githubusercontent.com/sims1253/ry/main/$path"
printf '\n--- %s ---\n' "$path"
curl -fsSL "$url" | rg -n -C 3 'ParamSpec|default|required|has_default' | head -n 120 || true
done
printf '%s\n' '--- vendor parameter metadata counts ---'
python3 - <<'PY'
import json
from pathlib import Path
from collections import Counter
counts = Counter()
examples = {}
for path in Path('crates/ry-typeshed/vendor').glob('*/**.json'):
try:
root = json.loads(path.read_text())
except Exception:
continue
def walk(x):
if isinstance(x, dict):
if 'name' in x and ('required' in x or 'default' in x):
key = (x.get('required', '<absent>'), x.get('default', '<absent>'))
counts[key] += 1
examples.setdefault(key, (str(path), x.get('name')))
for value in x.values():
walk(value)
elif isinstance(x, list):
for value in x:
walk(value)
walk(root)
for key, count in sorted(counts.items(), key=lambda item: str(item[0])):
print(key, count, examples[key])
PYRepository: sims1253/ry
Length of output: 15825
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository metadata and schema-related files ---'
curl -fsSL 'https://api.github.com/repos/sims1253/ry/git/trees/main?recursive=1' |
jq -r '.tree[].path' |
grep -Ei 'schema|typeshed|catalog|readme|json' |
head -n 200
printf '%s\n' '--- upstream references to ParamSpec/default semantics ---'
for path in \
crates/ry-typeshed/src/lib.rs \
crates/ry-analysis/src/catalog_adapter.rs \
crates/ry-analysis/src/catalog.rs \
crates/ry-typeshed/vendor/SOURCE \
README.md
do
url="https://raw.githubusercontent.com/sims1253/ry/main/$path"
printf '\n--- %s ---\n' "$path"
curl -fsSL "$url" |
rg -n -C 3 'ParamSpec|default|required|has_default' |
head -n 120 || true
done
printf '%s\n' '--- vendor parameter metadata counts ---'
python3 - <<'PY'
import json
from pathlib import Path
from collections import Counter
counts = Counter()
examples = {}
for path in Path('crates/ry-typeshed/vendor').glob('*/**.json'):
try:
root = json.loads(path.read_text())
except Exception:
continue
def walk(x):
if isinstance(x, dict):
if 'name' in x and ('required' in x or 'default' in x):
key = (x.get('required', '<absent>'), x.get('default', '<absent>'))
counts[key] += 1
examples.setdefault(key, (str(path), x.get('name')))
for value in x.values():
walk(value)
elif isinstance(x, list):
for value in x:
walk(value)
walk(root)
for key, count in sorted(counts.items(), key=lambda item: str(item[0])):
print(key, count, examples[key])
PYRepository: sims1253/ry
Length of output: 15825
Preserve explicit no-default metadata.
default: Some(false) is a valid explicit no-default value. Use p.default.unwrap_or(!p.required) instead of p.default.is_some(). The current RY091 path reads required directly, so this conversion affects catalog consumers of ParameterSpec.has_default, not RY091.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-analysis/src/catalog_adapter.rs` around lines 26 - 32, Update
convert_param so ParameterSpec.has_default uses the explicit p.default value
when present, including Some(false), and otherwise derives it from the inverse
of p.required via unwrap_or(!p.required). Leave the name and variadic mappings
unchanged.
Comment on lines
+34
to
+64
| fn convert_evaluation(sig: &FunctionSig) -> Evaluation { | ||
| // Check for promise-capture / defusing modes. | ||
| let modes: Vec<&EvalMode> = sig.eval.values().collect(); | ||
| if modes.iter().any(|m| matches!(m, EvalMode::QuotedSymbol)) { | ||
| return Evaluation::PromiseCapture { | ||
| defusing: DefusingKind::Ensym, | ||
| }; | ||
| } | ||
| if modes | ||
| .iter() | ||
| .any(|m| matches!(m, EvalMode::QuotedExpression)) | ||
| { | ||
| return Evaluation::PromiseCapture { | ||
| defusing: DefusingKind::Enquos, | ||
| }; | ||
| } | ||
| if modes.iter().any(|m| matches!(m, EvalMode::CapturesPromise)) { | ||
| return Evaluation::PromiseCapture { | ||
| defusing: DefusingKind::Enquo, | ||
| }; | ||
| } | ||
| if modes.iter().any(|m| matches!(m, EvalMode::TidySelect)) { | ||
| return Evaluation::DataMask { | ||
| select: Some(SelectKind::ColumnSelect), | ||
| }; | ||
| } | ||
| if modes.iter().any(|m| matches!(m, EvalMode::DataMask)) { | ||
| return Evaluation::DataMask { select: None }; | ||
| } | ||
| Evaluation::Eager | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Per-parameter evaluation modes are collapsed into one function-level value.
sig.eval is a map from parameter name to EvalMode. convert_evaluation discards every key and returns a single Evaluation for the whole function. The first matching mode in the fixed priority order wins.
For a function where only expr is defused and the remaining parameters are eager, the result is PromiseCapture for the whole call. A consumer that reads is_defusing in crates/ry-analysis/src/effect.rs then treats every argument as defused. That is the opposite of the module goal in catalog.rs lines 7-8.
ParameterSpec has no evaluation field, so the IR cannot express per-parameter modes at all. Add one, or document the collapse and its precedence order explicitly at this function.
The modes vector on line 36 is also unnecessary. Call sig.eval.values().any(...) directly and drop the allocation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-analysis/src/catalog_adapter.rs` around lines 34 - 64, Update
convert_evaluation and the IR around ParameterSpec so sig.eval modes remain
associated with their parameter names instead of collapsing into one
function-level Evaluation; ensure effect.rs consumers can distinguish defused
and eager arguments. If per-parameter representation is intentionally
unsupported, document that limitation and the existing precedence order directly
in convert_evaluation, and replace the allocated modes vector with direct
sig.eval.values().any(...) checks.
Comment on lines
+66
to
+89
| fn convert_return_rule(sig: &FunctionSig) -> ReturnRule { | ||
| if sig.no_return { | ||
| return ReturnRule::Unknown; | ||
| } | ||
| match &sig.return_ { | ||
| ry_typeshed::ReturnSpec::Slot(_) => ReturnRule::Conditional, | ||
| ry_typeshed::ReturnSpec::Concrete(_) => ReturnRule::Fixed("typed".to_string()), | ||
| } | ||
| } | ||
|
|
||
| fn convert_flow_effect(sig: &FunctionSig) -> FlowEffect { | ||
| if sig.no_return { | ||
| return FlowEffect::NoReturn; | ||
| } | ||
| if sig.predicate.is_some() { | ||
| return FlowEffect::Predicate { | ||
| target: crate::catalog::PredicateTarget::FirstArg, | ||
| }; | ||
| } | ||
| if sig.assertion.is_some() { | ||
| return FlowEffect::Assertion { stop_on_fail: true }; | ||
| } | ||
| FlowEffect::None | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect predicate, assertion, and JsonRType shapes in ry-typeshed.
set -euo pipefail
rg -nP --type=rust -C 12 'struct\s+JsonRType|predicate:|assertion:|struct\s+\w*Predicate|struct\s+\w*Assertion' crates/ry-typeshed/src/lib.rsRepository: sims1253/ry
Length of output: 7751
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- adapter structure and implementation ---'
ast-grep outline crates/ry-analysis/src/catalog_adapter.rs
sed -n '1,230p' crates/ry-analysis/src/catalog_adapter.rs
printf '%s\n' '--- catalog IR definitions and consumers ---'
rg -n -C 10 'enum ReturnRule|enum FlowEffect|struct FunctionSemantics|PredicateTarget|stop_on_fail|ReturnRule::Fixed|FlowEffect::Predicate|FlowEffect::Assertion' crates/ry-analysis/src
printf '%s\n' '--- source type definitions and construction sites ---'
rg -n -C 12 'pub enum ReturnSpec|Concrete\(|PredicateSpec|AssertionSpec|subject_param|allow_null_param|allow_na_param' crates/ry-typeshed crates/ry-analysisRepository: sims1253/ry
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
from collections import Counter
for path in Path("crates/ry-typeshed/vendor").rglob("*.json"):
try:
data = json.loads(path.read_text())
except Exception:
continue
for section in ("functions", "s3_methods"):
entries = data.get(section, {})
values = entries.values() if isinstance(entries, dict) else entries
for entry in values:
if entry.get("predicate") is not None:
p = entry["predicate"]
print(f"{path}: predicate subject={p.get('subject_param')!r} target_mode={p.get('target', {}).get('mode')!r}")
if entry.get("assertion") is not None:
a = entry["assertion"]
print(
f"{path}: assertion subject={a.get('subject_param')!r} "
f"allow_null={a.get('allow_null_param')!r} allow_na={a.get('allow_na_param')!r} "
f"target_mode={a.get('target', {}).get('mode')!r}"
)
PY
printf '%s\n' '--- all PredicateTarget construction and interpretation ---'
rg -n -C 8 'PredicateTarget::|PredicateTarget|stop_on_fail' crates/ry-analysis --glob '*.rs'
printf '%s\n' '--- relevant catalog documentation ---'
sed -n '53,105p' crates/ry-analysis/src/catalog.rsRepository: sims1253/ry
Length of output: 7860
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
from collections import Counter
counts = Counter()
subjects = Counter()
for path in Path("crates/ry-typeshed/vendor").rglob("*.json"):
try:
data = json.loads(path.read_text())
except Exception:
continue
for section in ("functions", "s3_methods"):
entries = data.get(section, {})
values = entries.values() if isinstance(entries, dict) else entries
for sig in values:
if sig.get("no_return"):
counts["no_return"] += 1
if sig.get("predicate") is not None:
counts["predicate"] += 1
subjects[("predicate", sig["predicate"]["subject_param"])] += 1
if sig.get("assertion") is not None:
counts["assertion"] += 1
subjects[("assertion", sig["assertion"]["subject_param"])] += 1
if sig.get("predicate") is not None and sig.get("assertion") is not None:
counts["predicate_and_assertion"] += 1
if sig.get("no_return") and (
sig.get("predicate") is not None or sig.get("assertion") is not None
):
counts["no_return_with_flow_effect"] += 1
print("counts:", dict(counts))
print("subjects:", dict(subjects))
PY
printf '%s\n' '--- return-rule consumers ---'
rg -n -C 12 'ReturnRule::Fixed|match .*return_rule|return_rule' crates/ry-analysis/src --glob '*.rs'Repository: sims1253/ry
Length of output: 16914
Preserve typeshed semantics in the catalog adapter
- Map
ReturnSpec::Concrete(concrete)toReturnRule::Fixed(concrete.mode.clone()). The current"typed"value discards the validated return mode, and the test locks in the wrong value. - Derive
PredicateTargetfrompredicate.subject_param.FirstArgmatches the current vendor data, but the adapter ignores this field. AssertionSpechas no stop flag. Do not claim thatstop_on_failcan be mapped from it. Extend the IR or document the intentional loss ofsubject_param,target,allow_null_param,allow_na_param, andprovenance.- Document or reject the current precedence:
no_returnoverrides the return and flow effects, andpredicateoverridesassertion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-analysis/src/catalog_adapter.rs` around lines 66 - 89, The catalog
adapter must preserve typeshed semantics: update convert_return_rule to use
ReturnSpec::Concrete(concrete).mode.clone(), derive PredicateTarget from
sig.predicate.subject_param, and remove the unsupported AssertionSpec
stop_on_fail mapping by extending the IR or explicitly documenting the lost
assertion and predicate fields. Also document or reject the existing precedence
where no_return overrides other effects and predicate overrides assertion.
Comment on lines
+91
to
+104
| fn convert_binding_effect(sig: &FunctionSig) -> Option<BindingEffect> { | ||
| if !sig.injects.is_empty() { | ||
| let names: Vec<String> = sig | ||
| .injects | ||
| .iter() | ||
| .flat_map(|i| i.names.iter().cloned()) | ||
| .collect(); | ||
| if names.is_empty() { | ||
| return Some(BindingEffect::Inject(vec![])); | ||
| } | ||
| return Some(BindingEffect::Inject(names)); | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
The empty-names branch is redundant.
If names is empty, Some(BindingEffect::Inject(vec![])) and Some(BindingEffect::Inject(names)) are the same value. Lines 98-100 add a branch with no effect.
♻️ Proposed simplification
fn convert_binding_effect(sig: &FunctionSig) -> Option<BindingEffect> {
- if !sig.injects.is_empty() {
- let names: Vec<String> = sig
- .injects
- .iter()
- .flat_map(|i| i.names.iter().cloned())
- .collect();
- if names.is_empty() {
- return Some(BindingEffect::Inject(vec![]));
- }
- return Some(BindingEffect::Inject(names));
- }
- None
+ if sig.injects.is_empty() {
+ return None;
+ }
+ let names: Vec<String> = sig
+ .injects
+ .iter()
+ .flat_map(|i| i.names.iter().cloned())
+ .collect();
+ Some(BindingEffect::Inject(names))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn convert_binding_effect(sig: &FunctionSig) -> Option<BindingEffect> { | |
| if !sig.injects.is_empty() { | |
| let names: Vec<String> = sig | |
| .injects | |
| .iter() | |
| .flat_map(|i| i.names.iter().cloned()) | |
| .collect(); | |
| if names.is_empty() { | |
| return Some(BindingEffect::Inject(vec![])); | |
| } | |
| return Some(BindingEffect::Inject(names)); | |
| } | |
| None | |
| } | |
| fn convert_binding_effect(sig: &FunctionSig) -> Option<BindingEffect> { | |
| if sig.injects.is_empty() { | |
| return None; | |
| } | |
| let names: Vec<String> = sig | |
| .injects | |
| .iter() | |
| .flat_map(|i| i.names.iter().cloned()) | |
| .collect(); | |
| Some(BindingEffect::Inject(names)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-analysis/src/catalog_adapter.rs` around lines 91 - 104, Remove the
redundant names.is_empty() conditional in convert_binding_effect and return
Some(BindingEffect::Inject(names)) directly after collecting the names,
preserving the existing None behavior when sig.injects is empty.
Comment on lines
+86
to
+100
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub enum ReturnRule { | ||
| /// Always returns the given type. | ||
| Fixed(String), | ||
| /// Returns the type of the Nth argument (1-indexed). | ||
| NthArg(usize), | ||
| /// Returns the first non-null argument. | ||
| FirstNonNull, | ||
| /// Higher-order: maps over the Nth argument. | ||
| HigherOrder { map_over: usize }, | ||
| /// Conditional return based on arguments. | ||
| Conditional, | ||
| /// Cannot determine statically. | ||
| Unknown, | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Document the index base of HigherOrder::map_over, and consider a non-zero type.
NthArg documents "(1-indexed)". HigherOrder { map_over: usize } carries the same kind of argument position but documents no index base. A consumer that reads return_rule through crates/ry-analysis/src/effect.rs has no way to know which convention applies, which invites an off-by-one.
Lines 7-8 also state the design goal: "making impossible states unrepresentable". A 1-indexed position typed as usize still permits 0, which is not a valid argument position. std::num::NonZeroUsize encodes that constraint.
crates/ry-analysis/src/catalog_adapter.rs never constructs either variant today, so no current caller is affected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-analysis/src/catalog.rs` around lines 86 - 100, Update
ReturnRule::HigherOrder so map_over uses NonZeroUsize and explicitly documents
that the argument position is 1-indexed, matching NthArg. Import the type as
needed and adjust any affected pattern matches or consumers, while preserving
the existing return-rule behavior.
Comment on lines
+3032
to
+3054
| fn extract_prefix(text: &str, position: tower_lsp::lsp_types::Position) -> Option<String> { | ||
| let line = text.lines().nth(position.line as usize)?; | ||
| let byte_col = line | ||
| .char_indices() | ||
| .take(position.character as usize) | ||
| .last() | ||
| .map(|(i, _)| i) | ||
| .unwrap_or(0); | ||
| let before = &line[..byte_col]; | ||
| let prefix: String = before | ||
| .chars() | ||
| .rev() | ||
| .take_while(|c| c.is_alphanumeric() || *c == '_' || *c == '.') | ||
| .collect::<String>() | ||
| .chars() | ||
| .rev() | ||
| .collect(); | ||
| if prefix.is_empty() { | ||
| None | ||
| } else { | ||
| Some(prefix) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
extract_prefix drops the last character and uses the wrong position unit.
Two defects:
- Off-by-one.
line.char_indices().take(position.character as usize).last().map(|(i, _)| i)yields the byte index of the last included character, not the index after it. For the lineunique_withcharacter = 7,byte_colis 6, sobeforeis"unique"and the prefix loses the trailing_. Worse, after typing a single charactercharacteris 1,byte_colis 0,beforeis empty, and the function returnsNone. Completion then applies no prefix filter and offers every project function in the workspace. - Wrong unit.
position.characteris a UTF-16 code unit count, as advertised at line 751. This code countscharvalues. Any line containing an astral character before the cursor produces a wrong offset.
Reuse the existing conversion used everywhere else in this file, position_to_byte_offset_pos, which already handles UTF-16 and returns None for invalid positions.
🐛 Proposed fix
fn extract_prefix(text: &str, position: tower_lsp::lsp_types::Position) -> Option<String> {
- let line = text.lines().nth(position.line as usize)?;
- let byte_col = line
- .char_indices()
- .take(position.character as usize)
- .last()
- .map(|(i, _)| i)
- .unwrap_or(0);
- let before = &line[..byte_col];
+ let offset = position_to_byte_offset_pos(text, position)?;
+ let before = &text[..offset];
let prefix: String = before
.chars()
.rev()
.take_while(|c| c.is_alphanumeric() || *c == '_' || *c == '.')
.collect::<String>()
.chars()
.rev()
.collect();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn extract_prefix(text: &str, position: tower_lsp::lsp_types::Position) -> Option<String> { | |
| let line = text.lines().nth(position.line as usize)?; | |
| let byte_col = line | |
| .char_indices() | |
| .take(position.character as usize) | |
| .last() | |
| .map(|(i, _)| i) | |
| .unwrap_or(0); | |
| let before = &line[..byte_col]; | |
| let prefix: String = before | |
| .chars() | |
| .rev() | |
| .take_while(|c| c.is_alphanumeric() || *c == '_' || *c == '.') | |
| .collect::<String>() | |
| .chars() | |
| .rev() | |
| .collect(); | |
| if prefix.is_empty() { | |
| None | |
| } else { | |
| Some(prefix) | |
| } | |
| } | |
| fn extract_prefix(text: &str, position: tower_lsp::lsp_types::Position) -> Option<String> { | |
| let offset = position_to_byte_offset_pos(text, position)?; | |
| let before = &text[..offset]; | |
| let prefix: String = before | |
| .chars() | |
| .rev() | |
| .take_while(|c| c.is_alphanumeric() || *c == '_' || *c == '.') | |
| .collect::<String>() | |
| .chars() | |
| .rev() | |
| .collect(); | |
| if prefix.is_empty() { | |
| None | |
| } else { | |
| Some(prefix) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-lsp/src/backend.rs` around lines 3032 - 3054, Update extract_prefix
to use the existing position_to_byte_offset_pos conversion for the cursor
position, preserving UTF-16 handling and invalid-position behavior. Use the
returned byte offset directly as the slice boundary so the character at the
cursor is excluded while all preceding characters, including a trailing
underscore, remain in the extracted prefix.
| // tests can assert that the publish/hover/completion hot path performs no | ||
| // baseline file I/O. | ||
| pub use backend::baseline_disk_reads; | ||
| pub use backend::{COMPILE_DURING_LAST_PUBLISH, FILTER_COMPILE_COUNT}; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Export accessor functions instead of mutable statics.
This re-export makes two AtomicU64 statics part of the public API of ry-lsp. Any consumer can call store on them and change the values the test gate reads. The existing instrumentation in this crate uses an accessor for exactly this reason: baseline_disk_reads() (crates/ry-lsp/src/backend.rs lines 46-48) returns the value and keeps the static private.
Add read-only accessors and export those. This also composes with the per-server scoping requested on crates/ry-lsp/src/backend.rs lines 200-211.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-lsp/src/lib.rs` at line 154, Replace the public re-export of
COMPILE_DURING_LAST_PUBLISH and FILTER_COMPILE_COUNT with read-only accessor
functions in the backend module, following the existing baseline_disk_reads()
pattern. Re-export the accessors from lib.rs and keep the AtomicU64 statics
private so consumers can read values without mutating the test-gate state;
ensure the accessors support the requested per-server scoping.
Comment on lines
+32
to
+44
| async fn spawn_session(root: &Path) -> (Session, tokio::task::JoinHandle<()>) { | ||
| let (client_stream, server_stream) = tokio::io::duplex(128 * 1024); | ||
| let (client_reader, client_writer) = tokio::io::split(client_stream); | ||
| let (server_reader, server_writer) = tokio::io::split(server_stream); | ||
| let server = tokio::spawn(async move { | ||
| let _ = ry_lsp::run_with(server_reader, server_writer).await; | ||
| }); | ||
| let mut session = LspSession::new(client_reader, client_writer); | ||
| session.initialize(root).await.unwrap(); | ||
| // Wait for background indexing to populate disk_files. | ||
| tokio::time::sleep(std::time::Duration::from_secs(1)).await; | ||
| (session, server) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Replace the fixed indexing sleep with a bounded poll.
spawn_session sleeps one second unconditionally to wait for spawn_background_index to populate disk_files. Six tests call it, so the suite pays six seconds even when indexing finishes immediately. More importantly the sleep is an assumption, not a synchronization point: on a loaded CI runner indexing can take longer, and every test in this file then fails for a reason unrelated to the feature under test.
Poll the observable condition instead. Retry the request under a deadline until it returns a non-null result, or wait for the first textDocument/publishDiagnostics for an indexed file, which the server sends after the index completes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-lsp/tests/p38_feature_diff.rs` around lines 32 - 44, Replace the
unconditional one-second sleep in spawn_session with bounded polling of the
indexing completion signal. Repeatedly issue the relevant observable request, or
await the first textDocument/publishDiagnostics notification for an indexed
file, until it returns a non-null result; enforce a deadline and fail clearly if
indexing does not complete in time.
Comment on lines
+176
to
+191
| let locations = match &def { | ||
| Value::Array(arr) => arr.clone(), | ||
| Value::Object(obj) => vec![Value::Object(obj.clone())], | ||
| _ => Vec::new(), | ||
| }; | ||
|
|
||
| let target_uris: Vec<&str> = locations | ||
| .iter() | ||
| .filter_map(|l| l.get("uri").and_then(|u| u.as_str())) | ||
| .collect(); | ||
|
|
||
| assert!( | ||
| target_uris.iter().all(|uri| *uri != a_uri), | ||
| "definition should resolve to the local b.R helper, not a.R with the same spelling" | ||
| ); | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This assertion passes when no definition is returned.
locations is empty when textDocument/definition returns null, and Iterator::all on an empty iterator is true. So the test passes both when the definition correctly resolves to R/b.R and when the server returns no definition at all. It cannot detect a regression that breaks go-to-definition entirely.
Assert that at least one location exists and that it points at b_uri.
💚 Proposed fix
+ assert!(
+ !target_uris.is_empty(),
+ "definition must resolve to at least one location"
+ );
assert!(
target_uris.iter().all(|uri| *uri != a_uri),
"definition should resolve to the local b.R helper, not a.R with the same spelling"
);
+ assert!(
+ target_uris.iter().any(|uri| *uri == b_uri),
+ "definition should point at the local b.R helper"
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let locations = match &def { | |
| Value::Array(arr) => arr.clone(), | |
| Value::Object(obj) => vec![Value::Object(obj.clone())], | |
| _ => Vec::new(), | |
| }; | |
| let target_uris: Vec<&str> = locations | |
| .iter() | |
| .filter_map(|l| l.get("uri").and_then(|u| u.as_str())) | |
| .collect(); | |
| assert!( | |
| target_uris.iter().all(|uri| *uri != a_uri), | |
| "definition should resolve to the local b.R helper, not a.R with the same spelling" | |
| ); | |
| }) | |
| let locations = match &def { | |
| Value::Array(arr) => arr.clone(), | |
| Value::Object(obj) => vec![Value::Object(obj.clone())], | |
| _ => Vec::new(), | |
| }; | |
| let target_uris: Vec<&str> = locations | |
| .iter() | |
| .filter_map(|l| l.get("uri").and_then(|u| u.as_str())) | |
| .collect(); | |
| assert!( | |
| !target_uris.is_empty(), | |
| "definition must resolve to at least one location" | |
| ); | |
| assert!( | |
| target_uris.iter().all(|uri| *uri != a_uri), | |
| "definition should resolve to the local b.R helper, not a.R with the same spelling" | |
| ); | |
| assert!( | |
| target_uris.iter().any(|uri| *uri == b_uri), | |
| "definition should point at the local b.R helper" | |
| ); | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-lsp/tests/p38_feature_diff.rs` around lines 176 - 191, Update the
definition-result assertions around locations and target_uris to require at
least one returned location, then verify that a returned URI equals b_uri.
Preserve the existing exclusion of a_uri while ensuring the test fails when
textDocument/definition returns null or an empty array.
Comment on lines
+580
to
+593
| let mut skipped_count: u32 = 0; | ||
| for (step, operation) in operations.into_iter().enumerate() { | ||
| // P37-W7c: Track operations skipped due to invalid state. | ||
| // After precomputation, the generator should only produce | ||
| // valid operations, so this should remain zero. | ||
| let operation = if !model.is_valid(&operation) { | ||
| skipped_count += 1; | ||
| match model.valid_alternative(&operation) { | ||
| Some(valid) => valid, | ||
| None => continue, | ||
| } | ||
| } else { | ||
| operation | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
The counter name and the report disagree with what is counted.
skipped_count increments once for every invalid operation, whether valid_alternative returns a replacement (line 588) or the operation is dropped (line 589). The report at line 950 attributes all of them to corrections. So a run in which every invalid operation was dropped reports them as corrected.
Track the two outcomes separately and report both. Also note the string literal at line 950 contains a run of spaces from the line break, which reaches the output as valid alternatives.
♻️ Proposed refactor
- let mut skipped_count: u32 = 0;
+ let mut corrected_count: u32 = 0;
+ let mut dropped_count: u32 = 0;
for (step, operation) in operations.into_iter().enumerate() {
let operation = if !model.is_valid(&operation) {
- skipped_count += 1;
match model.valid_alternative(&operation) {
- Some(valid) => valid,
- None => continue,
+ Some(valid) => {
+ corrected_count += 1;
+ valid
+ }
+ None => {
+ dropped_count += 1;
+ continue;
+ }
}
} else {
operation
};- if skipped_count > 0 {
- eprintln!(
- "P37-W7c: {skipped_count} operations were corrected to valid alternatives (coverage could be improved with state-aware generation)"
- );
- }
+ if corrected_count > 0 || dropped_count > 0 {
+ eprintln!(
+ "P37-W7c: {corrected_count} operations corrected, {dropped_count} dropped; \
+ coverage could be improved with state-aware generation"
+ );
+ }Also applies to: 947-952
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ry-lsp/tests/w8_session.rs` around lines 580 - 593, Separate the
invalid-operation accounting in the operations loop from the `skipped_count`
variable into distinct counts for replacements and dropped operations: increment
the replacement count only when `valid_alternative` returns `Some`, and
increment the dropped count before `continue` for `None`. Update the report
around the existing reporting logic near `valid_alternative` to include both
outcomes with accurate labels, and remove the unintended whitespace from the
multiline string so it does not output padding between words.
6 tasks
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
2 participants
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.
Plans 37–39: Release preparation, analysis host, and semantic catalog
This PR implements all three plans (70 commits) on top of the reviewed baseline (
df1a4ef).Plan 37: Release truth and editor hardening ✅ (8/8)
sims1253.ry)Plan 38: One analysis host ✅ (12/12)
ry-diagnosticsexternal crate v0.1.0ry-config/ry-workspaceno longer depend onry-checker), CI gatery-analysiscrate withAnalysisHost,Change, stable identitiesAnalysisSnapshotwithQueryResult<T>, 3 proptest propertiesSymbolId, cross-fileSymbolIndexry_analysis::check_projectPlan 39: External semantic catalog ✅ (9/10)
r-typeshed-schemacrate, taggedschema-v0.1.0Key fixes in this PR
Project::force_full_recollection()fixesw10_session_converges_to_fresh_server(was pre-existing, verified at P37 baseline)Gates
ecosystem/check-cargo-edges.pydocs/architecture/External repos
ry-diagnostics: tagged v0.1.0 (not published)r-typeshed:schema-v0.1.0tag onschema-crate/Breaking changes
None. All changes are additive or internal restructuring. CLI/LSP diagnostic identity preserved.
Note
Add ry-analysis crate, semantic function catalog, and cross-file LSP features for Plans 37–39
ry-analysiscrate as a unified analysis host with anAnalysisHost/AnalysisSnapshotabstraction, acheck_projectentry point used by both CLI and LSP, a semantic function catalog (FunctionSemantics, catalog adapters from typeshed), and a cross-file symbol index.NamespaceMetadata, sentinels,PackageFileKind) and shared diagnostic types (Severity,Confidence,BaselineDiagnostic,FFI_PRIMITIVES) out ofry-checkerintory-workspaceandry-corerespectively;ry-configandry-workspaceno longer depend onry-checker.FolderAnalysisContext) instead of reconstructing them per file during diagnostics publication.strip_quotes_at_boundaries.RY_EXTENSION_IDupdated tosims1253.ryand version bumped to 0.9.0.build_filter/filter_from_configfromry-configtory-checkerand restructuring module boundaries is a breaking internal API change for any downstream crates consuming these symbols directly.Macroscope summarized 6154633.
Summary by CodeRabbit