Skip to content

feat(adapters): add LangChain export adapter & runner with multi-provider support#81

Open
AJAmit17 wants to merge 4 commits into
open-gitagent:mainfrom
AJAmit17:feat/langchain-rebased
Open

feat(adapters): add LangChain export adapter & runner with multi-provider support#81
AJAmit17 wants to merge 4 commits into
open-gitagent:mainfrom
AJAmit17:feat/langchain-rebased

Conversation

@AJAmit17
Copy link
Copy Markdown
Contributor

@AJAmit17 AJAmit17 commented May 13, 2026

Summary

This is a clean rebase of #16 onto current main. The original PR predated several merges that touched the same integration files (src/adapters/index.ts, src/commands/export.ts, src/commands/run.ts), making the diff unreadable. This branch is three focused commits on top of b3a057a with all conflicts resolved — no existing adapters or options were removed.

Original PR: #16 (superseded by this one — closed)
Closes: #2


Hey @shreyas-lyzr — this is the clean rebase you asked for. Quick summary of what changed since #16:


What this adds

LangChain as a first-class export + run target alongside the existing adapters (kiro, gitclaw, codex, etc.):

File Role
src/adapters/langchain.ts Converts agent.yaml → standalone Python script using LangChain agent framework
src/runners/langchain.ts Executes the generated script in an auto-managed Python venv at ~/.gitagent/langchain-venv
src/adapters/langchain.test.ts Unit tests for provider detection and export shape
docs/adapters/langchain.md Full field-mapping table, provider table, usage examples

Integration changes (additive only):

  • src/adapters/index.tsexportToLangChain added as a new export line
  • src/commands/export.tslangchain case added; format help string updated
  • src/commands/run.tslangchain case added; adapter help string updated

Also fixes: pre-existing TypeScript build failure from #79

PR #79 added financial_governance references in src/commands/validate.ts and src/adapters/shared.ts but the property was never declared on ComplianceConfig in src/utils/loader.ts, causing a TypeScript build failure on all Node versions in CI. This PR adds the missing type declaration and guards the unsafe <= 0 comparison that the strict types exposed.


Addressing reviewer questions from #16

Q: Is runWithLangChain invoking the user's installed Python LangChain (subprocess) or shipping a JS LangChain runtime?

Subprocess. The runner (src/runners/langchain.ts) bootstraps a Python venv at ~/.gitagent/langchain-venv on first run, installs the required langchain-openai or langchain-anthropic pip package depending on the detected provider, writes the generated Python script to a temp file, then invokes it via spawnSync. No JS LangChain runtime is shipped.

Q: Multi-provider switching — provider precedence and conflict behaviour?

Provider is determined solely from manifest.model.preferred (the model name string). The detection rules:

  • claude-* → Anthropic (langchain-anthropic, ANTHROPIC_API_KEY)
  • gpt-*, o1-*/o2-*/o3-*/o4-* → OpenAI (langchain-openai, OPENAI_API_KEY)
  • Anything else → hard error at export time (unsupported model)

The corresponding env var must be set at runtime; the runner validates and errors clearly if it is missing. There is no silent fallback between providers.

Q: Fidelity matrix row?

Happy to add a row to paper/tables/fidelity-matrix.csv in a follow-up, or I can include it here if preferred — just say the word.


Note on scope vs #80 (LangGraph + DeepAgents)

PR #80 added LangGraph (the state-machine layer on top of LangChain) and DeepAgents (a higher-level harness). This PR adds the base LangChain runtime. They compose — LangGraph imports from LangChain — so this is the foundational piece, not a duplicate.


Test plan

  • npm run build passes (Node 18 / 20 / 22)
  • gitagent export --format langchain -d <agent-dir> produces a valid Python script
  • gitagent run -a langchain -d <agent-dir> -p "hello" bootstraps venv and executes
  • Provider detection unit tests pass (langchain.test.ts)
  • Manually validated on Windows 11, Node 22, Python 3.12 with OpenAI and Anthropic models
  • All existing adapter cases untouched (kiro, gitclaw, codex, cursor, gemini, etc.)
  • ComplianceConfig type fix unblocks CI for all open PRs targeting main

Adds LangChain as a new export/run target alongside existing adapters
(kiro, gitclaw, codex, etc.) — no existing adapters removed or modified.

- src/adapters/langchain.ts: exports agent YAML → standalone Python script
  using LangChain agent framework; multi-provider support (OpenAI / Anthropic)
  via model-name prefix detection; tool stubs via @tool decorators; sub-agent
  delegation; system prompt from SOUL.md + RULES.md + skills
- src/runners/langchain.ts: runs an agent via a temporary Python venv;
  bootstraps pip deps, writes the generated script, executes it
- src/adapters/langchain.test.ts: unit tests for provider detection and export
- docs/adapters/langchain.md: field-mapping table, provider table, usage examples

Integration wired into:
- src/adapters/index.ts — export added (additive)
- src/commands/export.ts — 'langchain' case added; format list updated
- src/commands/run.ts   — 'langchain' case added; adapter list updated
AJAmit17 added 3 commits May 13, 2026 23:49
…gap from open-gitagent#79)

PR open-gitagent#79 added financial_governance references in validate.ts and shared.ts
but the property was never declared on ComplianceConfig in loader.ts,
causing a TypeScript build failure on all Node versions in CI.
…pen-gitagent#74)

- docs/adapters/langchain.md: gitagent <verb> → gapman <verb> in all CLI
  examples; @shreyaskapale/gitagent → @open-gitagent/gapman package ref
- src/adapters/langchain.ts: 'Generated by gitagent export' →
  'Generated by gapman export' (user-visible string in generated scripts)
- src/runners/langchain.ts: venv path gitagent-env → langchain-venv
  (matches the path documented in the PR and adapter docs)

Prose references to 'gitagent' as the protocol/project name preserved
as per open-gitagent#74 policy.
Copy link
Copy Markdown
Contributor

@shreyas-lyzr shreyas-lyzr left a comment

Choose a reason for hiding this comment

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

Solid PR — well-documented rebase, good test coverage, and the additive-only policy on integration files is correctly honored. A few things worth addressing before merge:

Blocking: unsupported model hard-errors at export time
The provider detection hard-errors on Gemini, Llama, Mistral, and custom models. This will surprise users who have a working gitagent setup with a non-OpenAI/Anthropic model and just want to try the LangChain export. Consider a fallback path (e.g. treat unknown models as an OpenAI-compatible endpoint and warn) or at minimum make the error message list the supported model prefixes explicitly.

Blocking: venv bootstrap happens silently on first run
The runner installs packages into ~/.gitagent/langchain-venv on first run with no user-visible progress. spawnSync with no stdio forwarding means the user sees nothing while pip potentially runs for 30+ seconds. Forward stdio or print a "Installing LangChain dependencies..." notice before the spawn.

Suggestion: TypeScript build fix belongs in a separate PR
The ComplianceConfig.financial_governance type fix from #79 is a pre-existing build failure that affects all open PRs — it should be merged standalone first (or already was?) so it can be cherry-picked cleanly. Bundling it here makes the diff harder to review and means the fix only lands when this PR does.

Suggestion: fidelity matrix row
As mentioned in the PR body — adding the LangChain row to paper/tables/fidelity-matrix.csv would make this PR self-contained. If you prefer a follow-up, please open a tracking issue.

Minor: langchain.test.ts import path
Verify the test file imports from the compiled .js output path (or uses ts-jest / tsx) — several existing test files in this repo use .js extension imports. If langchain.test.ts uses .ts imports it may fail depending on the tsconfig moduleResolution setting.

Overall the architecture is sound and the rebase is clean. Resolve the two blocking points and this is mergeable.

Copy link
Copy Markdown
Contributor

@shreyas-lyzr shreyas-lyzr left a comment

Choose a reason for hiding this comment

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

Overall this is a well-structured addition. The rebase is clean and the diff is readable. A few things worth addressing before merge:

  1. detectProvider returns null for unsupported models and throws in exportToLangChain, but the thrown error surfaces as an unhandled exception in the CLI — the caller in export.ts has no try/catch around the langchain case. Either wrap the case "langchain": block in a try/catch and call error(...) + process.exit(1), or switch to returning a typed error instead of throwing.

  2. In langchain.ts, detectProvider is exported only to enable tests (// Make detectProvider available for tests). If this is an internal utility that tests need, consider moving it to a shared test-helper file or exporting it from a separate langchain-utils.ts rather than widening the public adapter API surface with an internal function.

  3. The exportToLangChain function returns a string (the generated Python script), but export.ts calls it as result = exportToLangChain(dir) — the other adapters use exportTo*String naming for the string-returning variant. The naming inconsistency (no String suffix, no file-writing variant) is minor but breaks the established convention. Worth aligning.

  4. The langchain.ts provider detection regex /^o\d/ will match model names like o1, o3, o4-mini, which is correct, but it will also match a hypothetical o-chat model that starts with o followed by a digit. This is probably fine for now, but worth a comment explaining the intended coverage.

  5. The test file imports detectProvider as a named export — if the export is later removed or renamed, these tests will break silently at build time rather than at test time. A compile check is already implicit with TypeScript, so this is minor, just worth being aware of.

No blocking correctness issues in the generated Python output. The recursion_limit multiplication by 2 is a reasonable heuristic and the comment explains the intent.

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.

2 participants