Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the package from committed lib/ JavaScript artifacts to a TypeScript source build (src/ -> dist/), and introduces a new multi-agent API built on top of the Branch/BranchStore primitives.
Changes:
- Switch build + publish output from
lib/todist/(tsc declarations, updated package entrypoints, workflow adjustments). - Add new TS implementations for
Branch,BranchStore,Session, and agent helpers (forkAgent,runAgents). - Port examples/tests execution to TS via
tsxand update documentation references.
Reviewed changes
Copilot reviewed 34 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| typedoc.json | Updates TypeDoc entrypoint to dist/index.d.ts. |
| tsconfig.json | Adds TypeScript compiler configuration emitting to dist/. |
| test/examples.ts | Replaces examples integration runner with a TS/tsx version and adds deep-research coverage. |
| test/examples.js | Removes legacy JS examples runner. |
| src/types.ts | Updates public types; adds agent-related types and internal native binding surface. |
| src/index.ts | New TS entrypoint implementing loadBinary/createContext and exporting new APIs. |
| src/Session.ts | Adds session helper around trunk lifecycle and conversation deltas. |
| src/BranchStore.ts | Adds TS BranchStore implementation backed by native store calls. |
| src/Branch.ts | Adds TS Branch implementation (fork/prefill/commit/metrics/etc). |
| src/Agent.ts | Adds agent fork + batched run loop with tool execution integration. |
| package.json | Switches main/types to dist/, adds TS build scripts and tsx-based test/example scripts. |
| package-lock.json | Adds typescript, tsx, and @types/node dependencies/lock entries. |
| lib/index.js | Removes legacy JS entrypoint implementation. |
| lib/BranchStore.js | Removes legacy JS BranchStore implementation. |
| lib/Branch.js | Removes legacy JS Branch implementation. |
| examples/streaming/streaming.mjs | Removes legacy streaming example. |
| examples/streaming/streaming-tsampler.mjs | Removes legacy streaming+tsampler example. |
| examples/streaming/streaming-summary.mjs | Removes legacy streaming+summary example. |
| examples/streaming/README.md | Removes streaming examples documentation. |
| examples/speculative/speculative.mjs | Removes speculative decoding example. |
| examples/speculative/README.md | Removes speculative decoding documentation. |
| examples/grammar/grammar.mjs | Removes grammar branching example. |
| examples/grammar/README.md | Removes grammar branching documentation. |
| examples/entropy/entropy.ts | Updates entropy example to TS/tsx and dist/ imports. |
| examples/embed/embed.ts | Updates embedding example to TS/tsx and dist/ imports. |
| examples/chat/chat.ts | Updates chat example to TS/tsx and dist/ imports. |
| examples/best-of-n/best-of-n.mjs | Removes best-of-n example. |
| examples/best-of-n/README.md | Removes best-of-n documentation. |
| README.md | Updates docs source reference from lib/index.d.ts to src/types.ts. |
| .npmignore | Ensures TS sources/tsconfig are excluded from npm package. |
| .gitignore | Adds dist/ to ignored build outputs. |
| .github/workflows/tests.yml | Updates package-content assertions for dist/. |
| .github/workflows/release.yml | Updates runtime smoke-test imports from ./lib to ./dist. |
| .github/workflows/gpu-test.yml | Removes lib/** from path trigger set (TS sources now). |
| .github/workflows/docs.yml | Updates docs trigger paths to src/*. |
Comments suppressed due to low confidence (1)
examples/chat/chat.ts:37
createContextoptions usesthreads, but the public option isnThreads(perContextOptions). Withstrictthis should fail to typecheck, and at runtime the native addon will ignore the unknown field. Rename tonThreads.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async prefillUser(content: string, opts: { tools?: string } = {}): Promise<void> { | ||
| const sep = this._ctx.getTurnSeparator(); | ||
| const fmtOpts = opts.tools ? { tools: opts.tools } : {}; | ||
| const { prompt } = await this._ctx.formatChat( | ||
| JSON.stringify([{ role: 'system', content: '' }, { role: 'user', content }]), | ||
| fmtOpts | ||
| ); | ||
| const delta = await this._ctx.tokenize(prompt, false); | ||
| await this._trunk!.prefill([...sep, ...delta]); | ||
| } |
There was a problem hiding this comment.
prefillUser / prefillToolResult dereference this._trunk! without a guard. If a consumer forgets to set session.trunk (or disposes it), this throws a non-obvious error. Add an explicit check and throw a clear message (or accept a Branch parameter) before calling prefill().
src/Session.ts
Outdated
| ]) | ||
| ); | ||
| const delta = await this._ctx.tokenize(prompt, false); | ||
| await this._trunk!.prefill([...sep, ...delta]); |
There was a problem hiding this comment.
prefillToolResult also uses this._trunk! without verifying trunk is set/not disposed, which can surface as a confusing runtime error. Add an explicit guard with a helpful error message before calling prefill().
| await this._trunk!.prefill([...sep, ...delta]); | |
| if (!this._trunk || this._trunk.disposed) { | |
| throw new Error( | |
| 'Cannot prefill tool result: trunk branch is not set or has been disposed. ' + | |
| 'Ensure Session.trunk is assigned and not disposed before calling prefillToolResult().' | |
| ); | |
| } | |
| await this._trunk.prefill([...sep, ...delta]); |
typedoc.json
Outdated
| "$schema": "https://typedoc.org/schema.json", | ||
| "plugin": ["typedoc-rhineai-theme"], | ||
| "entryPoints": ["lib/index.d.ts"], | ||
| "entryPoints": ["dist/index.d.ts"], |
There was a problem hiding this comment.
TypeDoc entryPoints now targets dist/index.d.ts, but the docs workflow installs deps with --ignore-scripts and does not run tsc, so this file won't exist and npm run docs will fail. Point TypeDoc at TS sources (e.g. src/index.ts) or update the docs workflow to run the TS build before TypeDoc.
| child.stdout!.on('data', (data: Buffer) => { | ||
| const lines: string[] = data.toString().split('\n'); | ||
| for (const line of lines) { | ||
| if (line.startsWith('{')) { | ||
| try { | ||
| const event: ExampleEvent = JSON.parse(line); | ||
| events.push(event); | ||
| } catch { | ||
| // Ignore malformed JSON | ||
| } | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
JSONL parsing from stdout assumes each data chunk contains whole newline-delimited JSON objects. Node streams can split a JSON line across chunks, which will make JSON.parse fail and permanently drop events (flaky tests). Keep a persistent buffer per process, append chunks, split on \n, and only parse complete lines (carry the last partial line forward).
2d7d2e8 to
c1bd8af
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 58 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (3)
examples/chat/chat.ts:24
__dirnameis not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Useimport.meta.urlinstead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
examples/entropy/entropy.ts:28
__dirnameis not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Useimport.meta.urlinstead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
examples/embed/embed.ts:24
__dirnameis not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Useimport.meta.urlinstead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const DEFAULT_MODEL = path.resolve( | ||
| __dirname, | ||
| '../../models/Qwen3-4B-Instruct-2507-Q4_K_M.gguf' | ||
| ); | ||
| const DEFAULT_RERANKER = path.resolve( | ||
| __dirname, | ||
| '../../models/qwen3-reranker-0.6b-q4_k_m.gguf' | ||
| ); |
There was a problem hiding this comment.
__dirname is not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Use import.meta.url instead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
examples/deep-research/tasks/plan.ts
Outdated
| import * as path from 'node:path'; | ||
| import { Branch } from '../../../dist/index.js'; | ||
| import type { SessionContext } from '../../../dist/index.js'; | ||
|
|
There was a problem hiding this comment.
__dirname is not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Use import.meta.url instead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
| import * as path from 'node:path'; | |
| import { Branch } from '../../../dist/index.js'; | |
| import type { SessionContext } from '../../../dist/index.js'; | |
| import * as path from 'node:path'; | |
| import { fileURLToPath } from 'node:url'; | |
| import { Branch } from '../../../dist/index.js'; | |
| import type { SessionContext } from '../../../dist/index.js'; | |
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); |
| import * as path from 'node:path'; | ||
| import { Branch, BranchStore } from '../../../dist/index.js'; | ||
| import type { SessionContext } from '../../../dist/index.js'; | ||
|
|
There was a problem hiding this comment.
__dirname is not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Use import.meta.url instead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
| import * as path from 'node:path'; | |
| import { Branch, BranchStore } from '../../../dist/index.js'; | |
| import type { SessionContext } from '../../../dist/index.js'; | |
| import * as path from 'node:path'; | |
| import { fileURLToPath } from 'node:url'; | |
| import { Branch, BranchStore } from '../../../dist/index.js'; | |
| import type { SessionContext } from '../../../dist/index.js'; | |
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); |
| import type { ExecuteToolFn } from '../tools/types.js'; | ||
|
|
There was a problem hiding this comment.
__dirname is not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Use import.meta.url instead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
| import type { ExecuteToolFn } from '../tools/types.js'; | |
| import type { ExecuteToolFn } from '../tools/types.js'; | |
| import { fileURLToPath } from 'node:url'; | |
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); |
examples/deep-research/tasks/eval.ts
Outdated
| import type { SessionContext } from '../../../dist/index.js'; | ||
|
|
There was a problem hiding this comment.
__dirname is not defined in ES modules when using TypeScript with tsx. This will cause a runtime error. Use import.meta.url instead:
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));Or use import.meta.dirname (Node.js 20.11+).
| import type { SessionContext } from '../../../dist/index.js'; | |
| import type { SessionContext } from '../../../dist/index.js'; | |
| import { fileURLToPath } from 'node:url'; | |
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); |
| @@ -0,0 +1,339 @@ | |||
| /** | |||
There was a problem hiding this comment.
Typo in PR title: "Migartion" should be "Migration"
| include(FetchContent) | ||
| FetchContent_Declare( | ||
| md4c | ||
| GIT_REPOSITORY https://github.com/mity/md4c | ||
| GIT_TAG release-0.5.2 | ||
| ) |
There was a problem hiding this comment.
The FetchContent_Declare block for md4c pulls third-party C code directly from GitHub using a mutable tag (release-0.5.2), so a compromised or rewritten tag could silently inject malicious code into your native addon during CI/build and at runtime. To harden the supply chain, pin md4c to an immutable reference (e.g., a specific commit SHA or content hash) and/or vendor or mirror the source in a controlled location for builds that have access to secrets or produce production artifacts.
- Decompose monolithic handleQuery and tui into composable operations and view handlers connected by a Channel. Operations (plan, research, verify, evaluate, respond, etc.) are plain Operation<T> generators that compose via yield*/spawn — no linear phase/step assumptions.
- Fix structured concurrency bug in useAgentPool where try/finally only wrapped provide(), not the tick loop — if the tick loop threw, agent branches were never pruned. Move try/finally to wrap the entire agent lifecycle from creation through provide.
- Rewrite research prompt to use unconditional sequential process instead of conditional logic ("if zero results, simplify"). The 4B model doesn't follow conditionals but reliably follows checklists. Simple keyword greps first, completeness-check grep last. Improves recall from 2-4/5 to 5/5 on dorpus benchmark.
- Add SC integration tests (test/agents.ts) verifying ensure lifecycle, branch cleanup on normal/error/tool-error paths via initAgents.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 68 out of 71 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (6)
test/examples.ts:1
- The examples runner references TypeScript example paths (e.g. 'entropy/entropy.ts', 'embed/embed.ts'), but this PR only shows updates to *.mjs files under examples/. As-is, test/examples.ts will fail with “file not found” unless the examples were renamed/added to .ts. Fix by either (a) renaming/adding the example files to .ts to match these paths, or (b) switching the EXAMPLES paths back to the actual filenames.
src/Rerank.ts:1 - Rerank imports createContext from './index.js' while src/index.ts imports and re-exports Rerank, creating a circular dependency that can make createContext undefined at runtime depending on module initialization order. Fix by moving createContext/loadBinary into a separate module (e.g., src/core.ts) that both src/index.ts and src/Rerank.ts import from, or by injecting the context factory into Rerank.create instead of importing from index.
test/examples.ts:1 - JSONL parsing here is not chunk-safe: stdout 'data' events can split a single JSON object across chunks, causing valid events to be dropped as “malformed JSON”. Fix by buffering partial lines across chunks (keep a trailing remainder string, append new data, split on '\n', parse complete lines only).
src/agents/agent-pool.ts:1 - ensureReportTool() will throw a raw JSON.parse error if a consumer passes an invalid toolsJson string in AgentTaskSpec.tools. Since toolsJson is part of the public API surface (string), it would be better to catch parse errors and throw a clearer message indicating the expected format (e.g., JSON array of tool schemas) and which agent/task failed.
src/agents/agent-pool.ts:1 - Cleanup in finally prunes all branches unconditionally and without isolation. If prune() throws for one branch, remaining branches won’t be pruned, which defeats the “always clean up” intent. Fix by guarding disposed branches and wrapping each prune in its own try/catch so cleanup continues for the rest.
src/Util.cpp:1 - Hard-coding md4c parser.abi_version to 0 is fragile and obscures intent. Prefer using the library constant (e.g., MD_PARSER_ABI_VERSION) so this stays correct if md4c’s ABI version changes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
README.md
Outdated
| Full API documentation: **[lloyal-ai.github.io/lloyal.node](https://lloyal-ai.github.io/lloyal.node/)** | ||
|
|
||
| Generated from [`lib/index.d.ts`](./lib/index.d.ts) with TypeDoc. | ||
| Generated from [`src/types.ts`](./src/types.ts) with TypeDoc. |
There was a problem hiding this comment.
This line says docs are generated from src/types.ts, but typedoc.json now uses entryPoints: ["src/index.ts"]. Update the README to match the actual TypeDoc entrypoint (or adjust typedoc.json if src/types.ts is the intended source).
| Generated from [`src/types.ts`](./src/types.ts) with TypeDoc. | |
| Generated from [`src/index.ts`](./src/index.ts) with TypeDoc. |
…b-agents Replace graceThreshold/criticalThreshold with two-concept pressure model: - softLimit: remaining KV floor for new work, enforced at SETTLE (reject tool results), PRODUCE (deny non-terminal tools), and INIT (drop agents) - hardLimit: crash-prevention floor, kill before produceSync() Fix reporter sub-agents trapped in infinite rejection loop: - hasNonTerminalTools checked pool registry instead of agent's tools, blocking reporters from calling report() as first action - Reporter pool now receives report-only tool registry - Research softLimit=2048 reserves KV for reporter suffix prefill (~350 tokens per reporter from chat template + tools JSON) Also: keep stderr alive with --trace flag, agreement scoring, report task prompt, TUI sub-agent rendering.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 71 out of 74 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
package.json:1
- @types/node@^25.3.0
andtypescript@^5.9.3do not correspond to versions I’m aware of and will likely break installs in CI/users. Please pin these to published versions (e.g., align ``@types/node`` to the supported Node major(s) for this repo, and settypescript` to a known-released version), or use a range that matches an actually-released major.
src/Rerank.ts:1 - src/Rerank.ts
importscreateContextfrom./index.js, butsrc/index.tsalso imports/exportsRerank. This creates a circular dependency that can leavecreateContextundefined or partially initialized at runtime. Consider movingcreateContext(or the minimal binary-loading/context-creation logic) into a separate module (e.g.,src/createContext.ts) that bothindex.tsandRerank.ts` import, so neither needs to import the top-level barrel.
test/examples.ts:1 - The examples test runner now targets
.tsexample entrypoints (e.g.,entropy/entropy.ts,embed/embed.ts), but in this diff the corresponding example files shown are still.mjs(and currently contain TS-only syntax). This will causetest:examplesto fail to locate/run the scripts. Align the example filenames/paths (rename the example files to.tsor update the runner to point at the actual filenames).
test/examples.ts:1 - The examples test runner now targets
.tsexample entrypoints (e.g.,entropy/entropy.ts,embed/embed.ts), but in this diff the corresponding example files shown are still.mjs(and currently contain TS-only syntax). This will causetest:examplesto fail to locate/run the scripts. Align the example filenames/paths (rename the example files to.tsor update the runner to point at the actual filenames).
test/examples.ts:1 - The examples test runner now targets
.tsexample entrypoints (e.g.,entropy/entropy.ts,embed/embed.ts), but in this diff the corresponding example files shown are still.mjs(and currently contain TS-only syntax). This will causetest:examplesto fail to locate/run the scripts. Align the example filenames/paths (rename the example files to.tsor update the runner to point at the actual filenames).
test/examples.ts:1 - This JSONL parser can drop/ignore valid events when a JSON line is split across stdout chunks (common for large outputs or different buffering). Because partial lines won’t
JSON.parse(), they’re silently discarded, which can make the test flaky. Keep aremainderbuffer betweendataevents (append chunk text, split on\n, keep the last partial line as the new remainder) to ensure every full JSONL record is parsed exactly once.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.