Refactor

README.md

@@ -2,7 +2,7 @@
 
 LLM-driven pull-request reviewer. Posts inline findings, reconciles prior bot threads on follow-up pushes, and writes a summary verdict.
 
-- **VCS**: GitHub, GitLab, Bitbucket (pluggable; `mock://` for tests)
+- **VCS**: GitHub, GitLab, Bitbucket (pluggable; `local://` for console-only branch review, `mock://` for tests)
 - **LLM**: any OpenAI-compatible `/v1` endpoint (Anthropic, OpenAI, vLLM, Ollama, OpenRouter, Groq, …)
 - **Runtime**: ships as a self-contained Docker image — consumers don't need Node or pnpm
 
@@ -36,15 +36,17 @@ The workflow builds the reviewer image from source on each run and triggers on e
 
 For Node projects, consume the reviewer as a `devDependency` instead of building the image — no Docker required. Works with npm, yarn, or pnpm.
 
-Add it to the consumer project, pinned to a tag (the git form is the same for every package manager):
+Add it to the consumer project from the npm registry:
 
 ```json
 "devDependencies": {
-  "@reviewer/cli": "github:bilikaz/code-reviewer#v0.1.1"
+  "@bilikaz/code-reviewer": "^0.2.0"
 }
 ```
 
-The package ships prebuilt JavaScript — `dist/` is committed — so the install does no build step (Node refuses to run TypeScript from inside `node_modules`, so compiled output is shipped instead). Then copy [examples/review.yml](examples/review.yml) into `<your-project>/.github/workflows/` and set the same `LLM_URL` / `LLM_KEY` / `LLM_MODEL` secrets as above. If this reviewer repo is **private**, the consumer's install step needs a PAT — see the comments in that file.
+(A git-tag form also works for unpublished revisions — `"@bilikaz/code-reviewer": "github:bilikaz/code-reviewer#<tag>"` — because `dist/` is committed: prebuilt JavaScript, no install-time build. Node refuses to run TypeScript from inside `node_modules`, so compiled output is shipped instead. A private repo consumed this way needs a PAT in the install step.)
+
+Then copy [examples/review.yml](examples/review.yml) into `<your-project>/.github/workflows/` and set the same `LLM_URL` / `LLM_KEY` / `LLM_MODEL` secrets as above.
 
 Run it locally the same way:
 
@@ -106,12 +108,13 @@ All config is env-only and read once at startup. See [.env.example](.env.example
 | Variable | Required | Default | Notes |
 | --- | --- | --- | --- |
 | `LLM_URL` | yes | — | OpenAI-compatible base URL |
-| `LLM_KEY` | yes | — | empty allowed for self-hosted endpoints with no auth |
+| `LLM_KEY` | no | — | omit for self-hosted endpoints with no auth (Authorization header is skipped) |
 | `LLM_MODEL` | no | auto-detect from `/models` | |
 | `GITHUB_TOKEN` / `GITLAB_TOKEN` / `BITBUCKET_TOKEN` | one | — | the one matching the PR URL |
 | `LLM_TEMPERATURE` | no | `0.2` | |
 | `LLM_MAX_OUTPUT_TOKENS` | no | `32768` | |
 | `LLM_HEAL_RETRIES` | no | `2` | retries when LLM output fails schema validation |
+| `LLM_DEBUG` | no | off | `1`/`true` dumps the full request body per LLM call (very chatty) |
 
 ## Development
 
@@ -138,11 +141,17 @@ src/
     reconcile/        re-judge prior bot threads
     review/           LLM review + inline findings
     summarize/        final verdict + summary comment
-  llm/                OpenAI-compatible streaming client, tool loop, schema healing
-  vcs/                github / gitlab / bitbucket / mock providers
-  lib/                git diff parsing
-  logger/             structured JSON logging
+  llm/                client (callLLM + resolveModel, schema healing), transport (HTTP + SSE), tools (Read/Bash)
+  providers/          Provider port + github / gitlab / bitbucket / local / mock adapters
+  lib/                host-level helpers (git diffs, error normalization)
+  logger/             Logger port + console / in-memory sinks
 tests/
   e2e/                vitest end-to-end tests
   fixtures/           mock old/new repo pairs per scenario
+docs/
+  ARCHITECTURE.md     the map: structure, patterns, flows
+  conventions/        portable engineering rules (naming, types placement, …)
+  adr/                dated architecture decision records
 ```
+
+Documentation is layered: [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) is this repo's map, [docs/conventions/](docs/conventions/) holds the portable rule set (reusable across projects), and [docs/adr/](docs/adr/) is the decision log that adopts them.

dist/cli.js

@@ -20,10 +20,12 @@
 //   --llm-key <key>     overrides LLM_KEY
 //   --llm-model <name>  overrides LLM_MODEL (otherwise auto-detected)
 import { buildCtx } from "./ctx.js";
+import { errorMessage } from "./lib/errors.js";
 import { runFetch } from "./stages/fetch/index.js";
 import { runReconcile } from "./stages/reconcile/index.js";
 import { runReview } from "./stages/review/index.js";
 import { runSummarize } from "./stages/summarize/index.js";
+import { isOpenBotThread } from "./providers/types.js";
 const FLAG_ALIASES = {
     "--pr": "pr",
     "--llm-url": "llmUrl",
@@ -69,7 +71,7 @@ function buildOverrides(flags) {
 }
 // ---- Pipeline orchestration --------------------------------------------
 function hasUnresolvedBotThreads(state) {
-    return state.comments.some((c) => c.by === "bot" && c.inline && !c.resolved && !c.parentId);
+    return state.comments.some(isOpenBotThread);
 }
 async function runCommand(command, ctx) {
     switch (command) {
@@ -113,6 +115,6 @@ try {
         process.exit(1);
 }
 catch (e) {
-    ctx.logger.error("cli.fatal", { error: e.message });
+    ctx.logger.error("cli.fatal", { error: errorMessage(e) });
     process.exit(1);
 }

dist/ctx.js

@@ -1,7 +1,7 @@
 // Ctx — dependency-injection container. Built once at CLI entry, passed to
 // every stage. Holds:
 //   - config: immutable settings (all env reads + CLI overrides live here)
-//   - vcs:    instantiated provider (already authenticated)
+//   - provider: instantiated review-platform adapter (already authenticated)
 //   - logger: structured logger (swap sinks here)
 //
 // Single rule: process.env is read ONLY in loadConfig(). Everything
@@ -10,15 +10,17 @@
 // .env loading.
 import { existsSync } from "node:fs";
 import { ConsoleLogger } from "./logger/console.js";
-import { resolveModel } from "./llm/model.js";
-import { BitbucketProvider } from "./vcs/bitbucket.js";
-import { GitHubProvider } from "./vcs/github.js";
-import { GitLabProvider } from "./vcs/gitlab.js";
-import { LocalVcsProvider } from "./vcs/local.js";
-import { MockVcsProvider } from "./vcs/mock.js";
-import { detectProvider } from "./vcs/types.js";
+import { resolveModel } from "./llm/client.js";
+import { BitbucketProvider } from "./providers/bitbucket.js";
+import { GitHubProvider } from "./providers/github.js";
+import { GitLabProvider } from "./providers/gitlab.js";
+import { LocalProvider } from "./providers/local.js";
+import { MockProvider } from "./providers/mock.js";
+import { detectProvider } from "./providers/types.js";
 // ---- Defaults ------------------------------------------------------------
-const REVIEW_DEFAULTS = {
+// Exported so tests build their configs from the real defaults instead of
+// re-typing them (see docs/conventions/configuration.md).
+export const REVIEW_DEFAULTS = {
     diffFilter: {
         includeExtensions: [],
         fullFileThresholdLines: 800,
@@ -46,6 +48,9 @@ function asNumber(v, fallback) {
     const n = Number(v);
     return Number.isFinite(n) ? n : fallback;
 }
+function asBool(v) {
+    return v === "1" || (v ?? "").toLowerCase() === "true";
+}
 function requireField(value, name, hint) {
     if (!value)
         throw new Error(`missing ${name}. ${hint}`);
@@ -77,7 +82,7 @@ export function loadConfig(opts) {
             // models will simply finish well under the cap.
             maxOutputTokens: asNumber(e.LLM_MAX_OUTPUT_TOKENS, 32_768),
             healRetries: asNumber(e.LLM_HEAL_RETRIES, 2),
-            debug: pick(e.LLM_DEBUG) === "1" || pick(e.LLM_DEBUG).toLowerCase() === "true",
+            debug: asBool(e.LLM_DEBUG),
         },
         github: { token: pick(e.GITHUB_TOKEN, e.GH_TOKEN) },
         gitlab: { token: pick(e.GITLAB_TOKEN), baseUrl: pick(e.GITLAB_BASE_URL) },
@@ -115,20 +120,20 @@ export function loadConfig(opts) {
     return config;
 }
 // ---- VCS construction (plain dispatch — no env reads) -------------------
-function makeVcs(c) {
+function makeProvider(c) {
     switch (c.pr.provider) {
         case "github": return GitHubProvider.create(c);
         case "gitlab": return GitLabProvider.create(c);
         case "bitbucket": return BitbucketProvider.create(c);
-        case "mock": return MockVcsProvider.create(c);
-        case "local": return LocalVcsProvider.create(c);
+        case "mock": return MockProvider.create(c);
+        case "local": return LocalProvider.create(c);
     }
 }
 // ---- buildCtx (thin orchestrator) ---------------------------------------
 export async function buildCtx(opts) {
     const config = loadConfig(opts);
-    const [vcs] = await Promise.all([
-        makeVcs(config),
+    const [provider] = await Promise.all([
+        makeProvider(config),
         (async () => {
             if (!config.llm.model) {
                 config.llm.model = await resolveModel({
@@ -140,7 +145,7 @@ export async function buildCtx(opts) {
     ]);
     return {
         config,
-        vcs,
+        provider,
         logger: opts.logger ?? new ConsoleLogger(),
     };
 }

dist/lib/errors.js

@@ -0,0 +1,16 @@
+// Error-message normalization (see docs/conventions/error-handling.md). `catch (e)` binds unknown;
+// `(e as Error).message` lies when a non-Error was thrown (yields
+// undefined and the log line loses its cause). Every catch site that
+// needs the message goes through this helper instead.
+export function errorMessage(e) {
+    if (e instanceof Error)
+        return e.message;
+    if (typeof e === "string")
+        return e;
+    try {
+        return JSON.stringify(e);
+    }
+    catch {
+        return String(e);
+    }
+}