Skip to content

Conversation

@djzager
Copy link
Member

@djzager djzager commented Oct 10, 2025

Fixes #239

Summary by CodeRabbit

  • Documentation
    • Added comprehensive design doc for distributed language extensions in the Konveyor VSCode ecosystem.
    • Describes core orchestrator and language-specific extensions, provider-based coordination, IPC and data-flow for analysis.
    • Includes architecture diagram, release checklist, answered design questions, implementation sketches, lifecycle and activation notes, security risks/mitigations, migration guidance, and alternative approaches.

@coderabbitai
Copy link

coderabbitai bot commented Oct 10, 2025

Walkthrough

Adds a new README detailing a distributed language-extensions architecture for Konveyor's VSCode ecosystem: core orchestrator plus language-specific extensions that spawn external provider subprocesses, IPC with kai-analyzer-rpc and language servers, provider registration/metadata, workflows, implementation sketches, security notes, and open questions.

Changes

Cohort / File(s) Change summary
Docs: Distributed language extensions proposal
enhancements/distributed-language-extensions/README.md
Adds a comprehensive design document describing a distributed architecture (core orchestrator + language extensions + external providers), provider API and registration, kai-analyzer-rpc redesign, language-extension responsibilities (e.g., konveyor-java spawning java-external-provider and LSP proxying), detailed data flow and diagrams, release checklist, security/risks/mitigations, repo layout, alternatives, and open questions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as VSCode User
  participant Core as Konveyor Core Extension
  participant LangExt as Language Extension (e.g., konveyor-java)
  participant Prov as External Provider (java-external-provider)
  participant RPC as kai-analyzer-rpc
  participant LSP as Language Server (JDTLS)

  rect rgba(230,245,255,0.6)
    U->>Core: Open workspace / activate
    Core->>LangExt: registerProvider(config, assets)
    LangExt-->>Core: provider metadata & assets
  end

  rect rgba(235,255,235,0.6)
    U->>Core: Start analysis
    Core->>RPC: Analyze(workspace, provider config)
    RPC->>Prov: Spawn/connect (pipes) and forward Evaluate requests
    Prov->>LSP: Proxy LSP requests (if needed)
    LSP-->>Prov: Responses
    Prov-->>RPC: Analysis results
    RPC-->>Core: Findings, diagnostics
    Core-->>LangExt: Deliver language-specific hooks/results
    Core-->>U: Aggregate & surface results
  end

  rect rgba(255,240,230,0.6)
    alt Error / incompatibility
      RPC-->>Core: Error
      Core-->>LangExt: Notify provider
      Core-->>U: Surface error/status
    else Success
      Core-->>U: Done
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibble plans and trace each thread,
Core commands, the providers spread.
Pipes hum softly, LSP sings,
Rules and assets, tiny springs.
A rabbit hops where code is led. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "distributed language extensions for vscode" directly and specifically refers to the main change in this pull request—a comprehensive architectural design document for breaking up the monolithic core IDE extension into a distributed, language-agnostic system with language-specific extensions. The title is concise, clear, and would allow teammates scanning history to quickly understand the primary focus. While the title includes an emoji which could be considered stylistic noise, the core message is specific and directly aligned with the changeset content.
Linked Issues Check ✅ Passed The PR directly addresses all three core requirements from linked issue #239. It proposes breaking up the monolithic core IDE extension into a core orchestrator extension plus language-specific extensions, eliminating Java-centered assumptions by introducing a multi-language distributed architecture with external provider subprocesses, and enabling support for additional programming languages through a general-purpose provider registration and IPC mechanism. The architectural design document includes implementation notes for multiple languages (Java and Python) and establishes the framework for language-independent IDE extension development.
Out of Scope Changes Check ✅ Passed All changes in this PR are directly scoped to addressing the requirements in issue #239. The PR consists entirely of architectural documentation for distributed language extensions, including the core orchestrator design, language-specific extension patterns, IPC mechanisms, and multi-language support patterns. There are no unrelated changes or tangential modifications; every section of the README focuses on enabling the core IDE extension breakdown and supporting multiple programming languages through the proposed distributed architecture.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e28fca6 and ecdadcb.

📒 Files selected for processing (1)
  • enhancements/distributed-language-extensions/README.md (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
enhancements/distributed-language-extensions/README.md

99-99: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


405-405: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


505-505: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (6)
enhancements/distributed-language-extensions/README.md (6)

1-61: Architecture summary and open questions are well-structured and responsive to prior feedback.

The frontmatter, checklist, and open questions sections provide clear direction. Notably, the concrete answers address previous reviewer concerns (eager provider startup for configuration/ruleset queries, graceful failure with retry, GitHub release distribution). The summary effectively communicates the rationale for decomposition.


63-93: Problem statement and design goals are clearly articulated with appropriate scope boundaries.

The motivation section pinpoints concrete pain points (duplicate JDTLS, monolithic binary, tight coupling), and goals directly address them. Non-goals prevent scope creep and clarify that rule semantics remain stable. This positions the architecture to satisfy the PR objective of decoupling language support from the core analyzer.


179-225: Communication channels are well-defined with clear data flow.

Each of the four communication boundaries (core↔analyzer, core↔java extension, java extension↔provider, analyzer↔provider) has an explicit purpose and message schema. The step-by-step analysis flow concretely demonstrates how a Java rule evaluation traverses the distributed system with LSP proxying. This design effectively decouples language-specific logic from the core analyzer.


226-389: Implementation sketches clearly define API contracts and component responsibilities.

The TypeScript and Go code examples illustrate the intended control flow: konveyor-java registers with core API, spawns the provider subprocess, and proxies LSP requests; kai-analyzer-rpc accepts provider configs and connects to external providers; java-external-provider acts as both server (for Evaluate calls) and client (for LSP proxy requests). These sketches provide sufficient clarity for implementation without prescribing exact APIs. Error handling patterns are included.


391-399: Security and risk section appropriately focuses on operational concerns.

The identified risks (subprocess management, pipe communication failures, binary distribution) are relevant to the distributed architecture. Mitigations are concrete. The section omits the "Extension Trust Chain" risk flagged in prior review, which per past comments was addressed (since diagnostics are already exposed via vscode.diagnostics, provider access doesn't expand attack surface). Consider whether a brief note clarifying this non-risk would help future readers.


535-549: Verify no leftover HTML reviewer notes remain beyond the visible file range.

Past review feedback flagged a leftover HTML comment around lines 646–647 (e.g., <!-- Need to review what Claude wrote here) that should be removed before merge. The current file excerpt ends at line 549. If lines beyond 549 exist in the full file, please verify that this HTML comment has been removed.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a97fb56 and d264389.

📒 Files selected for processing (1)
  • enhancements/distributed-language-extensions/README.md (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
enhancements/distributed-language-extensions/README.md

115-115: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


213-213: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


371-371: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


600-600: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if this is helpful or not

Signed-off-by: David Zager <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFE] Break up core IDE extension and revisit java-centered assumptions on the core flow and UX

2 participants