feat(import): read the MR's review threads into the --from-mr extraction (#1187)

CHANGELOG.md

@@ -24,6 +24,7 @@ All notable changes to this project will be documented in this file. See [standa
 
 ### ✨ Features
 
+- `teamai import --from-mr` and `teamai ci extract-mr` read the MR's review threads into the extraction: GitLab discussions, GitHub review comments, reviews and conversation comments, and TGit notes, without system notes or teamai's own CI comments. The threads get a section of the prompt with its own size budget, each with the file and line it is anchored to, whether it is resolved, and its notes marked as the MR author's or a reviewer's; threads with a reply come first, a long thread is shortened to its first and last notes, and the model is told that a decision in a resolved thread outweighs a restatement of the diff. A failure to read the threads warns and extracts without them. `--no-discussions` keeps the previous behaviour. With `language: en` in `teamai.yaml` the learning is written in English, with English headings; otherwise it stays Chinese (for [#1187](https://github.com/Tencent/teamai-cli/issues/1187)).
 - A tool without the `teamai-recall` subagent (Pi, Hermes, OpenClaw) now gets a recall block too, which tells the agent to run `teamai recall "<keywords>"` itself before code, debugging or design work, with the same skip conditions as the subagent block. It shares the subagent block's markers, so `recall disable`, uninstall and doctor treat both alike. Pi's README row now shows ✓ for learnings, codebase and teamwiki (for [#945](https://github.com/Tencent/teamai-cli/issues/945)).
 - A YAML agent can set `model: strong`, `model: fast`, or an alias the team defines in an optional `models/aliases.yaml`, which maps each alias per tool to that tool's own model value and an optional effort. `teamai pull` writes the mapped model into every tool's agent file, and the effort into that tool's own field: `effort` for Claude, claude-internal, tclaude, CodeBuddy, Qoder and Qoder CN, `model_reasoning_effort` for Codex, codex-internal and tcodex, `variant` for OpenCode. Cursor receives its model string as written, so its bracket form (`claude-opus-5[effort=high]`) carries the effort; Copilot receives the first entry as one model string. Copilot, Cursor, Kiro, WorkBuddy, JoyCode, ZCode and OMP get no effort field: an effort mapped for them is dropped, and pull warns once, naming the alias and the tool. The `-internal` and `t` variants use the `claude` or `codex` entry and Qoder CN the `qoder` entry unless the alias has their own key; no other tool inherits one. A tool the alias does not map, or any tool when the team has no aliases file, gets no `model` field and runs on its default, never a literal `strong`. `tool_extras.<tool>.model` skips the alias for that tool, and an extras effort alone overrides the alias's effort. A `model` that is not a string now makes the agent fail to parse; a legacy `.md` agent whose `model` is an alias is copied as is with a warning. While `models/aliases.yaml` cannot be read, pull holds every agent that sets a `model`, since the file may define any name, and push skips them. Pull records the model each agent copy received, so an ordinary pull applies a changed resolution even when the team repo has not moved, which fixes an agent an older CLI wrote with `model: strong` as is, and delivers a copy that is missing; a copy the member edited is kept, and that pull names it with how to take the new model. Pull warns when an alias an agent resolved through is removed, also where the alias gave a tool no model field (for [#830](https://github.com/Tencent/teamai-cli/issues/830)).
 - A member overrides a team model alias on their own machine in `~/.teamai/models/aliases.yaml`, in the same `aliases:` shape. An entry replaces the team's whole entry for that tool, effort included, and `~` or `default` gives the tool no model field and no effort; only `tool_extras.<tool>.model` wins over it. Keys are `strong`, `fast` or an alias the team defines, so a member can map `strong` before the team has an aliases file, and any other name has no effect. The file applies in every scope and to every team using that alias name, and an ordinary pull applies an edit to it. While it cannot be read, pull holds the agents whose `model` is an alias, naming the file, and push skips them; since the file can make no name an alias, agents with a concrete model are delivered and pushed as usual. In the team file, `default` is written as a model value and `~` is an error. When pull keeps an edited copy whose deployed version changed, it now says `the version teamai would deploy there ... has changed since` instead of blaming the team, since the change may be the member's override (for [#830](https://github.com/Tencent/teamai-cli/issues/830)).

docs/guide/knowledge.md

@@ -126,6 +126,8 @@ teamai import --from-repo https://github.com/org/repo --skip-enrich
 
 `--from-mr` takes several MR URLs. Each draft is still confirmed on its own (or none with `--all`), the accepted learnings are published once at the end, and each repository's teamwiki is updated once. An MR that fails does not stop the rest; the run ends with a summary and exits non-zero if any failed.
 
+The extraction reads the MR's review threads along with its description, commits and diff: GitLab discussions, GitHub review comments, reviews and conversation comments, TGit notes. Each thread goes into the prompt with the file and line it is anchored to, whether it is resolved, and its notes marked as the MR author's or a reviewer's; a decision in a resolved thread outweighs the diff. teamai's own CI comments are skipped. `--no-discussions` leaves the threads out, for `teamai ci extract-mr` too. With `language: en` in `teamai.yaml` the learning is written in English; otherwise in Chinese.
+
 `--incremental` re-extracts only the files changed since the last import; each codebase keeps that baseline in its own `evidence/code/<slug>/`. A remote repo's slug is `<provider>__<owner>__<repo>`, shared by `--from-repo`, `--from-mr` and `teamai ci extract-mr`; `--dir` uses the repo name.
 
 `--max-files` raises the cap on source files collected per repository (default 200). Files are collected key-file first; a source with more files is truncated, and the run warns with both counts instead of dropping the rest silently.

docs/guide/zh-CN/knowledge.md

@@ -126,6 +126,8 @@ teamai import --from-repo https://github.com/org/repo --skip-enrich
 
 `--from-mr` 可以一次传多个 MR URL：每个草稿照旧单独确认（`--all` 则都不问），接受的 learning 在最后一次性发布，每个仓库的 teamwiki 只更新一次。其中一个 MR 失败不影响其余，运行结束时给出汇总，有失败则退出码非零。
 
+提炼时除了 MR 的描述、提交和 diff，还会读取评审 threads：GitLab 的 discussions，GitHub 的 review 评论、review 结论和会话评论，TGit 的 notes。每个 thread 带上它锚定的文件和行、是否已 resolved，以及按顺序标为 MR 作者或评审者的各条 note 进入提示词；已 resolved 的 thread 里的决定比 diff 更重要。teamai 自己的 CI 评论会被跳过。`--no-discussions` 不读取 threads，`teamai ci extract-mr` 同样适用。`teamai.yaml` 设了 `language: en` 时 learning 用英文写，否则用中文。
+
 `--incremental` 只重新提取上次导入后改过的文件，基线随每个代码库存在它自己的 `evidence/code/<slug>/` 里。远端仓库的 slug 是 `<provider>__<owner>__<repo>`，`--from-repo`、`--from-mr` 和 `teamai ci extract-mr` 用的都是它；`--dir` 用仓库名。
 
 `--max-files` 调整单个仓库采集源码文件的上限（默认 200）。采集按关键文件优先；文件数超出上限时会截断，并输出同时带两个计数的警告，不再静默丢弃。

skill-data/core/references/commands.md

@@ -392,6 +392,7 @@ Generated: do not edit by hand. Regenerate with
   - `--dir <path>` — Extract code knowledge from a local directory (same as --from-repo but no clone)
   - `--from-claude` (hidden) — Scan Claude/Cursor rule directories (the Claude root's rules/ — ~/.claude or the recorded toolRoots.claude — and ~/.cursor/rules)
   - `--from-mr <url...>` — Extract learning from one or more merged MRs/PRs and trigger incremental teamwiki update
+  - `--no-discussions` — Leave the MR's review threads out of the extraction (with --from-mr)
   - `--from-iwiki <space-id-or-url>` — Import documents from iWiki Space ID or page URL (requires TAI_PAT_TOKEN)
   - `--resume` (hidden) — Resume an interrupted import session
   - `--all` — Accept all suggestions without interactive confirmation
@@ -454,3 +455,4 @@ Generated: do not edit by hand. Regenerate with
     - `--write-mode <mode>` — Write strategy: direct | pending-review
     - `--output <dir>` — Write artifacts to directory
     - `--individual-comments` — Post each suggestion as separate comment with reaction/resolve support
+    - `--no-discussions` — Leave the MR's review threads out of the extraction

src/__tests__/ci-extract-mr.test.ts

@@ -89,6 +89,18 @@ describe('ciExtractMr', () => {
     );
   });
 
+  it('hands its --comment-marker to the extraction so its own comments stay out of the review threads (#1187)', async () => {
+    await ciExtractMr({
+      url: 'https://github.com/org/repo/pull/1',
+      mode: 'comment',
+      commentMarker: '<!-- learning-bot -->',
+    });
+
+    expect(mockImportFromMR).toHaveBeenCalledWith(expect.objectContaining({
+      commentMarker: '<!-- learning-bot -->',
+    }));
+  });
+
   it('write 模式需要 --team-repo 参数', async () => {
     await expect(ciExtractMr({
       url: 'https://github.com/org/repo/pull/1',

src/__tests__/github-mr-fetch.test.ts

@@ -9,10 +9,30 @@ vi.mock('node:child_process', async (importOriginal) => ({
   ...await importOriginal<typeof import('node:child_process')>(),
   execSync: vi.fn(() => { throw new Error('gh unavailable'); }),
 }));
+vi.mock('../utils/logger.js', () => ({
+  log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), dim: vi.fn() },
+}));
+import { log } from '../utils/logger.js';
 
 const PR = '/repos/team/repo/pulls/42';
 const METADATA = { title: 'Fix retries', body: null, user: { login: 'dev' }, merged_at: null };
 
+/** The review threads (#1187): diff comments, review summaries, conversation. */
+const REVIEW_COMMENTS = [
+  { id: 1, body: 'Three retries hide a dead bucket.', path: 'src/up.ts', line: 42, user: { login: 'bob' } },
+  { id: 2, body: 'Unrelated nit', path: 'src/x.ts', line: null, original_line: 7, user: { login: 'carol' } },
+  { id: 3, body: 'Agreed, fail fast on 403.', path: 'src/up.ts', line: 42, in_reply_to_id: 1, user: { login: 'dev' } },
+];
+const REVIEWS = [
+  { body: '', state: 'APPROVED', user: { login: 'bob' } },
+  { body: 'Please split the retry policy out.', state: 'CHANGES_REQUESTED', user: { login: 'carol' } },
+  { body: 'draft', state: 'PENDING', user: { login: 'erin' } },
+];
+const CONVERSATION = [
+  { body: '<!-- teamai:ci-extract -->\n## TeamAI 知识提炼', user: { login: 'ci-bot' } },
+  { body: 'Merging after the split.', user: { login: 'dev' } },
+];
+
 function mockResponses(failedPath?: string, status = 200, body = '', responseError?: Error): string[] {
   const seen: string[] = [];
   const request = (options: https.RequestOptions, callback: (res: IncomingMessage) => void) => {
@@ -23,6 +43,9 @@ function mockResponses(failedPath?: string, status = 200, body = '', responseErr
     const failed = key === failedPath;
     const response = failed ? body : diff ? 'diff --git a/a b/a\n+fixed\n'
       : url === PR ? JSON.stringify(METADATA)
+      : url.startsWith(`${PR}/comments`) ? JSON.stringify(REVIEW_COMMENTS)
+      : url.startsWith(`${PR}/reviews`) ? JSON.stringify(REVIEWS)
+      : url.startsWith('/repos/team/repo/issues/42/comments') ? JSON.stringify(CONVERSATION)
       : JSON.stringify([{ sha: 'abc123', commit: { message: 'Fix retries\n\nDetails' } }]);
     const req = Object.assign(new EventEmitter(), {
       setTimeout: vi.fn(),
@@ -95,3 +118,34 @@ describe('GitHub PR REST fallback', () => {
     });
   });
 });
+
+describe('GitHub PR review threads (#1187)', () => {
+  it('groups diff comments into threads and adds review verdicts and conversation, without teamai comments', async () => {
+    mockResponses();
+    const mr = await fetchGitHubPR('https://github.com/team/repo/pull/42');
+    expect(mr.discussions).toEqual([
+      { file: 'src/up.ts', line: 42, notes: [{ author: 'bob', body: 'Three retries hide a dead bucket.' }, { author: 'dev', body: 'Agreed, fail fast on 403.' }] },
+      { file: 'src/x.ts', line: 7, notes: [{ author: 'carol', body: 'Unrelated nit' }] },
+      { notes: [{ author: 'carol', body: 'changes requested: Please split the retry policy out.' }] },
+      { notes: [{ author: 'dev', body: 'Merging after the split.' }] },
+    ]);
+  });
+
+  it('requests none of the list endpoints with discussions: false', async () => {
+    const seen = mockResponses();
+    const mr = await fetchGitHubPR('https://github.com/team/repo/pull/42', { discussions: false });
+    expect(mr).not.toHaveProperty('discussions');
+    expect(seen.filter((u) => /comments|reviews/.test(u))).toEqual([]);
+  });
+
+  it('keeps the extraction going, with a warning, when a list endpoint fails', async () => {
+    vi.mocked(log.warn).mockClear();
+    mockResponses(`${PR}/reviews?per_page=100&page=1`, 403, '{"message":"Forbidden"}');
+    const mr = await fetchGitHubPR('https://github.com/team/repo/pull/42');
+    expect(mr.title).toBe('Fix retries');
+    expect(mr).not.toHaveProperty('discussions');
+    expect(vi.mocked(log.warn).mock.calls.map(([m]) => String(m))).toEqual([
+      expect.stringMatching(/Review threads not read \(GitHub API error 403/),
+    ]);
+  });
+});

src/__tests__/gitlab-mr-discussions.test.ts

@@ -0,0 +1,111 @@
+/**
+ * `fetchGitLabMR` reads the MR's review threads from `/discussions` (#1187):
+ * every page, without system notes or teamai's own CI comments, with the file
+ * and line a diff thread is anchored to and whether it is resolved. An API
+ * error there keeps the extraction going without threads, and
+ * `discussions: false` does not call the endpoint at all.
+ */
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+vi.mock('../utils/logger.js', () => ({
+  log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), dim: vi.fn() },
+}));
+
+import { fetchGitLabMR } from '../providers/gitlab/mr-fetch.js';
+import { log } from '../utils/logger.js';
+
+const URL = 'https://gitlab.com/group/repo/-/merge_requests/7';
+const API = 'https://gitlab.com/api/v4/projects/group%2Frepo/merge_requests/7';
+
+const MR = { title: 'Retry uploads', description: 'd', author: { username: 'alice' }, merged_at: '2026-09-20T10:00:00Z' };
+
+const note = (author: string, body: string, extra: Record<string, unknown> = {}) => ({ body, author: { username: author }, ...extra });
+
+const PAGE_1 = [
+  { notes: [note('gitlab', 'changed the description', { system: true })] },
+  {
+    notes: [
+      note('bob', 'Fail fast on 403.', { resolvable: true, resolved: true, position: { new_path: 'src/up.ts', old_path: 'src/up.ts', new_line: 42, old_line: null } }),
+      note('alice', 'Done.', { resolvable: true, resolved: true }),
+    ],
+  },
+  { notes: [note('ci', '<!-- teamai:ci-extract -->\n## TeamAI 知识提炼')] },
+];
+const PAGE_2 = [
+  { notes: [note('carol', 'Deleted line was load-bearing.', { resolvable: true, resolved: false, position: { new_path: null, old_path: 'src/old.ts', new_line: null, old_line: 3 } })] },
+  { notes: [note('dave', 'LGTM')] },
+];
+
+type Handler = (url: string) => { status?: number; body: unknown; nextPage?: string };
+
+function stubFetch(handler: Handler): string[] {
+  const seen: string[] = [];
+  vi.stubGlobal('fetch', vi.fn(async (input: string) => {
+    seen.push(input);
+    const { status = 200, body, nextPage } = handler(input);
+    return {
+      ok: status >= 200 && status < 300,
+      status,
+      headers: new Headers(nextPage ? { 'x-next-page': nextPage } : {}),
+      json: async () => body,
+      text: async () => (typeof body === 'string' ? body : JSON.stringify(body)),
+    } as unknown as Response;
+  }));
+  return seen;
+}
+
+const base: Handler = (url) => {
+  if (url === `${API}`) return { body: MR };
+  if (url.startsWith(`${API}/commits`)) return { body: [{ id: 'abc', title: 'retry' }] };
+  if (url.startsWith(`${API}/changes`)) return { body: { changes: [{ diff: '+retry' }] } };
+  if (url.endsWith('/discussions?per_page=100&page=1')) return { body: PAGE_1, nextPage: '2' };
+  if (url.endsWith('/discussions?per_page=100&page=2')) return { body: PAGE_2 };
+  throw new Error(`unexpected ${url}`);
+};
+
+beforeEach(() => {
+  vi.stubEnv('GITLAB_TOKEN', 'secret');
+  vi.mocked(log.warn).mockClear();
+});
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.unstubAllEnvs();
+});
+
+describe('fetchGitLabMR review threads (#1187)', () => {
+  it('reads every page of discussions, mapping anchors and resolution, without system or teamai notes', async () => {
+    const seen = stubFetch(base);
+    const mr = await fetchGitLabMR(URL);
+    expect(seen.filter((u) => u.includes('/discussions'))).toHaveLength(2);
+    expect(mr.discussions).toEqual([
+      { file: 'src/up.ts', line: 42, resolved: true, notes: [{ author: 'bob', body: 'Fail fast on 403.' }, { author: 'alice', body: 'Done.' }] },
+      { file: 'src/old.ts', line: 3, resolved: false, notes: [{ author: 'carol', body: 'Deleted line was load-bearing.' }] },
+      { file: undefined, line: undefined, resolved: undefined, notes: [{ author: 'dave', body: 'LGTM' }] },
+    ]);
+    expect(mr.title).toBe('Retry uploads');
+    expect(mr.diff).toBe('+retry');
+  });
+
+  it('does not call the endpoint with discussions: false', async () => {
+    const seen = stubFetch(base);
+    const mr = await fetchGitLabMR(URL, { discussions: false });
+    expect(seen.some((u) => u.includes('/discussions'))).toBe(false);
+    expect(mr).not.toHaveProperty('discussions');
+  });
+
+  it('keeps the extraction going, with a warning, when the discussions call fails', async () => {
+    stubFetch((url) => (url.includes('/discussions') ? { status: 403, body: { message: 'Forbidden' } } : base(url)));
+    const mr = await fetchGitLabMR(URL);
+    expect(mr).not.toHaveProperty('discussions');
+    expect(mr.title).toBe('Retry uploads');
+    expect(vi.mocked(log.warn).mock.calls.map(([m]) => String(m))).toEqual([
+      expect.stringMatching(/Review threads not read \(GitLab API error 403\)/),
+    ]);
+  });
+
+  it('returns an empty list for an MR without review', async () => {
+    stubFetch((url) => (url.includes('/discussions') ? { body: [] } : base(url)));
+    const mr = await fetchGitLabMR(URL);
+    expect(mr.discussions).toEqual([]);
+  });
+});

src/__tests__/gitlab-provider.test.ts

@@ -652,12 +652,16 @@ describe('fetchGitLabMR', () => {
       if (String(url) === `${base}/changes`) {
         return new Response(JSON.stringify({ changes: [{ diff: '+fixed' }] }), { status: 200 });
       }
+      if (String(url).startsWith(`${base}/discussions`)) {
+        return new Response(JSON.stringify([]), { status: 200 });
+      }
       return new Response(JSON.stringify({ title: 'T', description: null, author: { username: 'a' } }), { status: 200 });
     }) as never;
 
     const mr = await fetchGitLabMR('https://gitlab.com/gitlab/g/sub/repo/-/merge_requests/7');
 
-    expect(seen).toEqual([base, `${base}/commits?per_page=50`, `${base}/changes`]);
+    // The review threads (#1187) come from the same instance root.
+    expect(seen).toEqual([base, `${base}/commits?per_page=50`, `${base}/changes`, `${base}/discussions?per_page=100&page=1`]);
     expect(mr.commits).toEqual([{ hash: 'abc123', message: 'Fix' }]);
     expect(mr.diff).toBe('+fixed');
   });